[Bug 97059] Tahiti+DRI3+Unity+Blender corruption
https://bugs.freedesktop.org/show_bug.cgi?id=97059 --- Comment #14 from Fabian Maurer --- The opengl calls blender make are faulty, since a trace doesn't render properly on windows with proprietary AMD drivers either. Though I still want to ask, could we get rid of the random contents? Under windows it's just black instead of drawing random images. To me at least that seems like the better approach. -- 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/3] drm/etnaviv: submit support for in-fences
On Fri, Mar 17, 2017 at 7:58 AM, Lucas Stach wrote: > Am Freitag, den 17.03.2017, 14:42 + schrieb Russell King - ARM > Linux: >> On Fri, Mar 17, 2017 at 03:10:21PM +0100, Lucas Stach wrote: >> > Am Donnerstag, den 16.03.2017, 12:05 +0100 schrieb Philipp Zabel: >> > > Hi Gustavo, >> > > >> > > On Mon, 2017-03-13 at 14:37 -0300, Gustavo Padovan wrote: >> > > [...] >> > > > I was thinking on some function that would iterate over all fences in >> > > > the fence_array and check their context. The if we find our own gpu >> > > > context in there we fail the submit. >> > > >> > > Why would we have to fail if somebody feeds us our own fences? Wouldn't >> > > it be enough to just wait if there are foreign fences in the array? >> > >> > Yes, skipping the wait if all fences are from our own context is an >> > optimization and it's certainly not an issue if someone feeds us our own >> > fences. >> >> Are you sure about that - what if we have two GPUs, a 2D and 3D GPU, >> and we're fed an etnaviv fence for one GPU when submitting to the >> other GPU. >> >> So we do end up being fed our own fences, and we have to respect them >> otherwise we lose inter-GPU synchronisation, and that will break >> existing userspace. >> > The etnaviv GPUs, while being on the same DRM device, have distinct > fence contexts. So the 3D GPU will consider a fence from the 2D GPU as > foreign and properly wait on it. > > It's only when we get an in fence that has been generated as an out > fence by one (or multiple) submits to the same GPU, that we are able to > skip the wait and enqueue the command without waiting for the fence to > signal. With regard to the 2D and 3D GPU case, it seems to me that a good example use case would be where the 3D GPU is used in Android for all the surface generation using OpenGL and then the 2D GPU would get used to composite all those surfaces together, leaving the 3D open to work on other stuff. As I understand it, the 2D GPU is much faster at 2D compositing than the 3D GPU would be, (not to mention less power hungry.) > > Regards, > Lucas > > ___ > etnaviv mailing list > etna...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/etnaviv ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 7/15] dt-bindings: display: sun4i: Add allwinner, tcon-channel property
On Thu, Mar 16, 2017 at 1:37 AM, Rob Herring wrote: > On Tue, Mar 07, 2017 at 09:56:26AM +0100, Maxime Ripard wrote: >> The Allwinner Timings Controller has two, mutually exclusive, channels. >> When the binding has been introduced, it was assumed that there would be >> only a single user per channel in the system. >> >> While this is likely for the channel 0 which only connects to LCD displays, >> it turns out that the channel 1 can be connected to multiple controllers in >> the SoC (HDMI and TV encoders for example). And while the simultaneous use >> of HDMI and TV outputs cannot be achieved, switching from one to the other >> at runtime definitely sounds plausible. >> >> Add an extra property, allwinner,tcon-channel, to specify for a given >> endpoint which TCON channel it is connected to, while falling back to the >> previous mechanism if that property is missing. > > I think perhaps TCON channels should have been ports rather than > endpoints. The fact that the channels are mutually exclusive can be > handled in the driver and doesn't really matter in the binding. How > painful would it be to rework things to move TCON channel 1 from port 0, > endpoint 1 to port 1? Having a separate output port for channel 1 was one option we discussed. However it wouldn't work well with the kernel's of_graph based crtc detection (drm_of_find_possible_crtcs / drm_of_find_possible_crtcs), which has the crtcs bound via the output port. As the logic is used by multiple drivers, I'm not sure it's easy to rework or test. Also, we still have to support old device trees using channel 1 from output port 0 endpoint 1. This is the TV encoder on sun5i (A10s/A13/R8). Regards ChenYu ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] drm/etnaviv: submit support for in-fences
On Fri, Mar 17, 2017 at 03:58:27PM +0100, Lucas Stach wrote: > Am Freitag, den 17.03.2017, 14:42 + schrieb Russell King - ARM > Linux: > > On Fri, Mar 17, 2017 at 03:10:21PM +0100, Lucas Stach wrote: > > > Am Donnerstag, den 16.03.2017, 12:05 +0100 schrieb Philipp Zabel: > > > > Hi Gustavo, > > > > > > > > On Mon, 2017-03-13 at 14:37 -0300, Gustavo Padovan wrote: > > > > [...] > > > > > I was thinking on some function that would iterate over all fences in > > > > > the fence_array and check their context. The if we find our own gpu > > > > > context in there we fail the submit. > > > > > > > > Why would we have to fail if somebody feeds us our own fences? Wouldn't > > > > it be enough to just wait if there are foreign fences in the array? > > > > > > Yes, skipping the wait if all fences are from our own context is an > > > optimization and it's certainly not an issue if someone feeds us our own > > > fences. > > > > Are you sure about that - what if we have two GPUs, a 2D and 3D GPU, > > and we're fed an etnaviv fence for one GPU when submitting to the > > other GPU. > > > > So we do end up being fed our own fences, and we have to respect them > > otherwise we lose inter-GPU synchronisation, and that will break > > existing userspace. > > > The etnaviv GPUs, while being on the same DRM device, have distinct > fence contexts. So the 3D GPU will consider a fence from the 2D GPU as > foreign and properly wait on it. > > It's only when we get an in fence that has been generated as an out > fence by one (or multiple) submits to the same GPU, that we are able to > skip the wait and enqueue the command without waiting for the fence to > signal. Sounds fine then. Thanks. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] drm/pl111: Initial drm/kms driver for pl111
On Fri, Mar 17, 2017 at 03:47:42PM -0700, Eric Anholt wrote: > This is a modesetting driver for the pl111 CLCD display controller > found on various ARM platforms such as the Versatile Express. The > driver has only been tested on the bcm911360_entphn platform so far, > with PRIME-based buffer sharing between vc4 and clcd. > > It reuses the existing devicetree binding, while not using quite as > many of its properties as the fbdev driver does (those are left for > future work). As we're multiplatform on ARM, and this is using the PL11x AMBA IDs, we must ensure that it's compatible with everything that the fbdev driver is compatible with... however, I suspect that's not worth the effort (unless Linus W wants it?) If we make it PL111 specific, then we don't need to handle Integrator CP, or the Versatile PB/AB weirdness. The only thing left is the power etc enable bits on Realview which uses the PL111. See the code for Realview in drivers/video/fbdev/amba-clcd-versatile.c. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] drm/etnaviv: submit support for in-fences
On Fri, Mar 17, 2017 at 03:10:21PM +0100, Lucas Stach wrote: > Am Donnerstag, den 16.03.2017, 12:05 +0100 schrieb Philipp Zabel: > > Hi Gustavo, > > > > On Mon, 2017-03-13 at 14:37 -0300, Gustavo Padovan wrote: > > [...] > > > I was thinking on some function that would iterate over all fences in > > > the fence_array and check their context. The if we find our own gpu > > > context in there we fail the submit. > > > > Why would we have to fail if somebody feeds us our own fences? Wouldn't > > it be enough to just wait if there are foreign fences in the array? > > Yes, skipping the wait if all fences are from our own context is an > optimization and it's certainly not an issue if someone feeds us our own > fences. Are you sure about that - what if we have two GPUs, a 2D and 3D GPU, and we're fed an etnaviv fence for one GPU when submitting to the other GPU. So we do end up being fed our own fences, and we have to respect them otherwise we lose inter-GPU synchronisation, and that will break existing userspace. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 4.1-next 02/15] drm/nouveau/display: Enable vblank irqs after display engine is on again.
From: Mario Kleiner
commit ff683df7bf34f90766a50c7e7454e219aef2710e upstream.
In the display resume path, move the calls to drm_vblank_on()
after the point when the display engine is running again.
Since changes were made to drm_update_vblank_count() in Linux 4.4+
to emulate hw vblank counters via vblank timestamping, the function
drm_vblank_on() now needs working high precision vblank timestamping
and therefore working scanout position queries at time of call.
These don't work before the display engine gets restarted, causing
miscalculation of vblank counter increments and thereby large forward
jumps in vblank count at display resume. These jumps can cause client
hangs on resume, or desktop hangs in the case of composited desktops.
Fix this Linux 4.4 regression by reordering calls accordingly.
Signed-off-by: Mario Kleiner
Cc: Ben Skeggs
Cc: ville.syrj...@linux.intel.com
Cc: daniel.vet...@ffwll.ch
Cc: dri-devel@lists.freedesktop.org
Reviewed-by: Daniel Vetter
Signed-off-by: Dave Airlie
Signed-off-by: Greg Kroah-Hartman
Signed-off-by: Ding Tianhong
---
drivers/gpu/drm/nouveau/nouveau_display.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c
b/drivers/gpu/drm/nouveau/nouveau_display.c
index 8670d90..4d0b32b 100644
--- a/drivers/gpu/drm/nouveau/nouveau_display.c
+++ b/drivers/gpu/drm/nouveau/nouveau_display.c
@@ -620,10 +620,6 @@ nouveau_display_resume(struct drm_device *dev, bool
runtime)
nv_crtc->lut.depth = 0;
}
- /* Make sure that drm and hw vblank irqs get resumed if needed. */
- for (head = 0; head < dev->mode_config.num_crtc; head++)
- drm_vblank_on(dev, head);
-
/* This should ensure we don't hit a locking problem when someone
* wakes us up via a connector. We should never go into suspend
* while the display is on anyways.
@@ -633,6 +629,10 @@ nouveau_display_resume(struct drm_device *dev, bool
runtime)
drm_helper_resume_force_mode(dev);
+ /* Make sure that drm and hw vblank irqs get resumed if needed. */
+ for (head = 0; head < dev->mode_config.num_crtc; head++)
+ drm_vblank_on(dev, head);
+
list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
struct nouveau_crtc *nv_crtc = nouveau_crtc(crtc);
--
1.9.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Mesa-dev] [RFC libdrm 0/2] Replace the build system with meson
On March 16, 2017 5:41:24 PM Emil Velikov wrote: On 17 March 2017 at 00:21, Dylan Baker wrote: Hi Emil, Quoting Emil Velikov (2017-03-16 16:35:33) While I can see you're impressed by Meson, I would kindly urge you to not use it here. As you look closely you can see that one could trivially improve the times, yet the biggest thing is that most of the code in libdrm must go ;-) Perhaps I wasn't clear enough, I don't really expect this to land ever. I sent it out more because I'd written it and it works and is a useful demonstration of meson+ninja performance. Obviously 20 seconds -> 5 seconds isn't a huge deal :); but in a larger project, consider that a 4x speedup would be 4 minutes to 1 minute, and that is a huge difference in time. You are still failing to see past your usecase. As said before - if there's any need to improve things say so. Note that you simply cannot apply the 1000x speedup in any situation. Yes, you can't just linearly apply any scaling factor. However, when you build mesa on a machine with a decent number of threads (I think our build machine for the CI system has 32 threads), autotools+make is slow as mud. Also, there's very little you can do to speed up configure since it's a pile of shell and perl that inherently runs single-threaded and is fairly complex due to mesa's complicated dependencies. As the port is not 1:1 wrt the autoconf one, the performance numbers above are comparing apples to oranges. I fail to see what I'm missing from meson that would have an effect on the times I reported. There are some files that are installed by autoconf that I didn't bother to install with meson (because I don't expect this to land). Since I didn't time installs, I don't see how it isn't an apples to apples comparison. You already (explicitly) mentioned some differences. Admittedly not a deal breaker. I understand that libdrm is a pessimal case for recursive-make since most sub folders contain < 5 C files, However, even if you were to flatten the make files meson+ninja would still be faster when you consider that meson configures and builds faster than autotools configures. That's correct. If is so concerned - they should slim down the configure.ac ;-) There are real limits as to what you can do there. If you/others are unhappy with the build times of libdrm - poke me on IRC. I will give you some easy tips on how to improve those. You have some good python knowledge - I would kindly urge you to improve/rewrite the slow and/or hacky python scripts we have in mesa. This is a topic that was mentioned multiple times, and a part where everyone will be glad to see some progress. Thanks Emil The real goal here is to do mesa (in case I didn't make that clear either), and the advantage for mesa is not just performance, it's that meson supports visual studio on windows; which means that we could hopefully not just get faster builds, but also replace both autotools and scons with a single build system. Yes that was more than clear. Yet it won't fly, I'm afraid. The VMWare people like their SCons, How much? I would really rather the VMWare people speak on behalf of VMWare. Meson is the single best shot we've ever had for replacing both with one build system. I'm sure the VMware people would like to have a build system that gets maintained by the community as a whole. and Meson is not a thing on neither BSD(s), Solaris (and derivatives) nor Android :-\ I have trouble bringing myself to care. The BSDs need to stop using 10 year old compilers. It can be made to work on Solaris and BSD if someone bothered to put a little work into it. Besides, given that large chunks of GNOME are switching they're going to have to make it work some day soon anyway. Android is a bit unfortunate. Mesa is one of the few projects that let's the Android people carry their build system in-tree and I would like to keep that going if it were practical. Dylan and I have talked about this a decent bit and one potential solution is to see if the meson people would accept an Android back-end. Then we would be down to a single build system (wouldn't that be nice). If there's something "slow" say what/where and we can improve upon things. You seems to be rewriting $world because someone sold you that A is the holy grail. I don't think that's fair. No, Meson is not the holy grail but it is the closest anyone has yet been able to come to a viable autotools replacement. Speed is only one aspect to this. Unifying the Linux and windows builds is also a significant advantage. Also, autotools is objectively terrible and having a build system that's modifiable be mere humans without the need for hours of pouring over documentation only to find that you did it wrong anyway is a definite plus. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
GEM allocation for para-virtualized DRM driver
Hello, I am writing a para-virtualized DRM driver for Xen hypervisor and it now works with DRM CMA helpers, but I would also like to make it work with non-contigous memory: virtual machine that the driver runs in can't guarantee that CMA is actually physically contigous (that is not a problem because of IPMMU and other means, the only constraint I have is that I cannot mmap with pgprot == noncached). So, I am planning to use *drm_gem_get_pages* + *shmem_read_mapping_page_gfp* to allocate memory for GEM objects (scanout buffers + dma-bufs shared with virtual GPU) Do you think this is the right approach to take? Thank you, Oleksandr ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 4.1-next 04/15] drm: Fix treatment of drm_vblank_offdelay in drm_vblank_on() (v2)
commit bb74fc1bf3072bd3ab4ed5f43afd287a63baf2d7 upstream.
drm_vblank_offdelay can have three different types of values:
< 0 is to be always treated the same as dev->vblank_disable_immediate
= 0 is to be treated as "never disable vblanks"
> 0 is to be treated as disable immediate if kms driver wants it
that way via dev->vblank_disable_immediate. Otherwise it is
a disable timeout in msecs.
This got broken in Linux 3.18+ for the implementation of
drm_vblank_on. If the user specified a value of zero which should
always reenable vblank irqs in this function, a kms driver could
override the users choice by setting vblank_disable_immediate
to true. This patch fixes the regression and keeps the user in
control.
v2: Only reenable vblank if there are clients left or the user
requested to "never disable vblanks" via offdelay 0. Enabling
vblanks even in the "delayed disable" case (offdelay > 0) was
specifically added by Ville in commit cd19e52aee922
("drm: Kick start vblank interrupts at drm_vblank_on()"),
but after discussion it turns out that this was done by accident.
Citing Ville: "I think it just ended up as a mess due to changing
some of the semantics of offdelay<0 vs. offdelay==0 vs.
disable_immediate during the review of the series. So yeah, given
how drm_vblank_put() works now, I'd just make this check for
offdelay==0."
Signed-off-by: Mario Kleiner
Reviewed-by: Daniel Vetter
Cc: mic...@daenzer.net
Cc: vba...@suse.cz
Cc: ville.syrj...@linux.intel.com
Cc: daniel.vet...@ffwll.ch
Cc: dri-devel@lists.freedesktop.org
Cc: alexander.deuc...@amd.com
Cc: christian.koe...@amd.com
Signed-off-by: Dave Airlie
Signed-off-by: Greg Kroah-Hartman
conflicts:
drivers/gpu/drm/drm_irq.c
Signed-off-by: Ding Tianhong
---
drivers/gpu/drm/drm_irq.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index af9662e..009e755 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -1308,8 +1308,7 @@ void drm_vblank_on(struct drm_device *dev, int crtc)
* re-enable interrupts if there are users left, or the
* user wishes vblank interrupts to be enabled all the time.
*/
- if (atomic_read(&vblank->refcount) != 0 ||
- (!dev->vblank_disable_immediate && drm_vblank_offdelay == 0))
+ if (atomic_read(&vblank->refcount) != 0 || drm_vblank_offdelay == 0)
WARN_ON(drm_vblank_enable(dev, crtc));
spin_unlock_irqrestore(&dev->vbl_lock, irqflags);
}
--
1.9.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Mesa-dev] [RFC libdrm 0/2] Replace the build system with meson
On Thu, Mar 16, 2017 at 8:03 PM, Jason Ekstrand wrote: > On March 16, 2017 5:41:24 PM Emil Velikov > wrote: > >> On 17 March 2017 at 00:21, Dylan Baker wrote: >> >>> Hi Emil, >>> >>> Quoting Emil Velikov (2017-03-16 16:35:33) >>> While I can see you're impressed by Meson, I would kindly urge you to not use it here. As you look closely you can see that one could trivially improve the times, yet the biggest thing is that most of the code in libdrm must go ;-) >>> >>> Perhaps I wasn't clear enough, I don't really expect this to land ever. >>> I sent >>> it out more because I'd written it and it works and is a useful >>> demonstration of >>> meson+ninja performance. Obviously 20 seconds -> 5 seconds isn't a huge >>> deal :); >>> but in a larger project, consider that a 4x speedup would be 4 minutes >>> to 1 >>> minute, and that is a huge difference in time. >>> >>> You are still failing to see past your usecase. As said before - if >> there's any need to improve things say so. >> Note that you simply cannot apply the 1000x speedup in any situation. >> > > Yes, you can't just linearly apply any scaling factor. However, when you > build mesa on a machine with a decent number of threads (I think our build > machine for the CI system has 32 threads), autotools+make is slow as mud. > Also, there's very little you can do to speed up configure since it's a > pile of shell and perl that inherently runs single-threaded and is fairly > complex due to mesa's complicated dependencies. > > As the port is not 1:1 wrt the autoconf one, the performance numbers above are comparing apples to oranges. >>> >>> I fail to see what I'm missing from meson that would have an effect on >>> the >>> times I reported. There are some files that are installed by autoconf >>> that I >>> didn't bother to install with meson (because I don't expect this to >>> land). Since >>> I didn't time installs, I don't see how it isn't an apples to apples >>> comparison. >>> >>> You already (explicitly) mentioned some differences. Admittedly not a >> deal breaker. >> >> I understand that libdrm is a pessimal case for recursive-make since most >>> sub folders contain < 5 C files, However, even if you were to flatten >>> the make >>> files meson+ninja would still be faster when you consider that meson >>> configures and builds faster than autotools configures. >>> >>> That's correct. If is so concerned - they should slim down the >> configure.ac ;-) >> > > There are real limits as to what you can do there. > > If you/others are unhappy with the build times of libdrm - poke me on IRC. I will give you some easy tips on how to improve those. You have some good python knowledge - I would kindly urge you to improve/rewrite the slow and/or hacky python scripts we have in mesa. This is a topic that was mentioned multiple times, and a part where everyone will be glad to see some progress. Thanks Emil >>> >>> The real goal here is to do mesa (in case I didn't make that clear >>> either), and >>> the advantage for mesa is not just performance, it's that meson supports >>> visual >>> studio on windows; which means that we could hopefully not just get >>> faster >>> builds, but also replace both autotools and scons with a single build >>> system. >>> >>> Yes that was more than clear. Yet it won't fly, I'm afraid. >> >> The VMWare people like their SCons, >> > > How much? I would really rather the VMWare people speak on behalf of > VMWare. Meson is the single best shot we've ever had for replacing both > with one build system. I'm sure the VMware people would like to have a > build system that gets maintained by the community as a whole. > Sure, I'd like to see one build system instead of two. Meson supports Windows so that's good. But the big issue is our automated build system. Replacing SCons with Meson could be a big deal in that context. It would at least involve pulling Meson into our toolchain and rewriting a bunch of Python code to grok Meson. I'd have to go off and investigate that to even see if it's a possibility. Maybe next week. -Brian > > and Meson is not a thing on neither BSD(s), Solaris (and derivatives) nor >> Android :-\ >> > > I have trouble bringing myself to care. The BSDs need to stop using 10 > year old compilers. It can be made to work on Solaris and BSD if someone > bothered to put a little work into it. Besides, given that large chunks of > GNOME are switching they're going to have to make it work some day soon > anyway. > > Android is a bit unfortunate. Mesa is one of the few projects that let's > the Android people carry their build system in-tree and I would like to > keep that going if it were practical. Dylan and I have talked about this a > decent bit and one potential solution is to see if the meson people would > accept an Android back-end. Then we would be down to a single build system > (wouldn't that be nice). > > If there's s
[Bug 97059] Tahiti+DRI3+Unity+Blender corruption
https://bugs.freedesktop.org/show_bug.cgi?id=97059 Michel Dänzer changed: What|Removed |Added CC||dark.shad...@web.de --- Comment #13 from Michel Dänzer --- *** Bug 100266 has been marked as a duplicate of this bug. *** -- 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 PATCHv2 21/21] staging: android: ion: Set query return value
This never got set in the ioctl. Properly set a return value of 0 on success. Signed-off-by: Laura Abbott --- drivers/staging/android/ion/ion.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c index 64c652b..8bd90ce 100644 --- a/drivers/staging/android/ion/ion.c +++ b/drivers/staging/android/ion/ion.c @@ -498,6 +498,7 @@ int ion_query_heaps(struct ion_heap_query *query) } query->cnt = cnt; + ret = 0; out: up_read(&dev->lock); return ret; -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC PATCHv2 18/21] staging: android: ion: Rework heap registration/enumeration
The current model of Ion heap registration is based on the outdated
model of board files. The replacement for board files (devicetree)
isn't a good replacement for what Ion wants to do. In actuality, Ion
wants to show what memory is available in the system for something else
to figure out what to use. Switch to a model where Ion creates its
device unconditionally and heaps are registed as available regions.
Currently, only system and CMA heaps are converted over to the new
model. Carveout and chunk heaps can be converted over when someone wants
to figure out how.
Signed-off-by: Laura Abbott
---
drivers/staging/android/ion/Kconfig | 25 +
drivers/staging/android/ion/Makefile| 7 +--
drivers/staging/android/ion/ion.c | 28 +--
drivers/staging/android/ion/ion.h | 40 +--
drivers/staging/android/ion/ion_carveout_heap.c | 10
drivers/staging/android/ion/ion_chunk_heap.c| 9
drivers/staging/android/ion/ion_cma_heap.c | 24 +++--
drivers/staging/android/ion/ion_heap.c | 67 -
drivers/staging/android/ion/ion_system_heap.c | 38 --
9 files changed, 85 insertions(+), 163 deletions(-)
diff --git a/drivers/staging/android/ion/Kconfig
b/drivers/staging/android/ion/Kconfig
index 15108c4..a517b2d 100644
--- a/drivers/staging/android/ion/Kconfig
+++ b/drivers/staging/android/ion/Kconfig
@@ -10,6 +10,31 @@ menuconfig ION
If you're not using Android its probably safe to
say N here.
+config ION_SYSTEM_HEAP
+ bool "Ion system heap"
+ depends on ION
+ help
+ Choose this option to enable the Ion system heap. The system heap
+ is backed by pages from the buddy allocator. If in doubt, say Y.
+
+config ION_CARVEOUT_HEAP
+ bool "Ion carveout heap support"
+ depends on ION
+ help
+ Choose this option to enable carveout heaps with Ion. Carveout heaps
+ are backed by memory reserved from the system. Allocation times are
+ typically faster at the cost of memory not being used. Unless you
+ know your system has these regions, you should say N here.
+
+config ION_CHUNK_HEAP
+ bool "Ion chunk heap support"
+ depends on ION
+ help
+ Choose this option to enable chunk heaps with Ion. This heap is
+ similar in function the carveout heap but memory is broken down
+ into smaller chunk sizes, typically corresponding to a TLB size.
+ Unless you know your system has these regions, you should say N here.
+
config ION_CMA_HEAP
bool "Ion CMA heap support"
depends on ION && CMA
diff --git a/drivers/staging/android/ion/Makefile
b/drivers/staging/android/ion/Makefile
index a892afa..eb7eeed 100644
--- a/drivers/staging/android/ion/Makefile
+++ b/drivers/staging/android/ion/Makefile
@@ -1,4 +1,5 @@
-obj-$(CONFIG_ION) += ion.o ion-ioctl.o ion_heap.o \
- ion_page_pool.o ion_system_heap.o \
- ion_carveout_heap.o ion_chunk_heap.o
+obj-$(CONFIG_ION) += ion.o ion-ioctl.o ion_heap.o
+obj-$(CONFIG_ION_SYSTEM_HEAP) += ion_system_heap.o ion_page_pool.o
+obj-$(CONFIG_ION_CARVEOUT_HEAP) += ion_carveout_heap.o
+obj-$(CONFIG_ION_CHUNK_HEAP) += ion_chunk_heap.o
obj-$(CONFIG_ION_CMA_HEAP) += ion_cma_heap.o
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index e1fb865..7d40233 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -40,6 +40,9 @@
#include "ion.h"
+static struct ion_device *internal_dev;
+static int heap_id = 0;
+
bool ion_buffer_cached(struct ion_buffer *buffer)
{
return !!(buffer->flags & ION_FLAG_CACHED);
@@ -1198,9 +1201,10 @@ static int debug_shrink_get(void *data, u64 *val)
DEFINE_SIMPLE_ATTRIBUTE(debug_shrink_fops, debug_shrink_get,
debug_shrink_set, "%llu\n");
-void ion_device_add_heap(struct ion_device *dev, struct ion_heap *heap)
+void ion_device_add_heap(struct ion_heap *heap)
{
struct dentry *debug_file;
+ struct ion_device *dev = internal_dev;
if (!heap->ops->allocate || !heap->ops->free)
pr_err("%s: can not add heap with invalid ops struct.\n",
@@ -1217,6 +1221,7 @@ void ion_device_add_heap(struct ion_device *dev, struct
ion_heap *heap)
heap->dev = dev;
down_write(&dev->lock);
+ heap->id = heap_id++;
/*
* use negative heap->id to reverse the priority -- when traversing
* the list later attempt higher id numbers first
@@ -1256,14 +1261,14 @@ void ion_device_add_heap(struct ion_device *dev, struct
ion_heap *heap)
}
EXPORT_SYMBOL(ion_device_add_heap);
-struct ion_device *ion_device_create(void)
+int ion_device_create(void)
{
struct ion_device *idev;
int ret;
idev = kzalloc(sizeof(*idev), GFP_KERNEL);
if (!idev)
- re
[RFC PATCHv2 20/21] staging: android: ion: Remove ion_handle and ion_client
ion_handle was introduced as an abstraction to represent a reference to
a buffer via an ion_client. As frameworks outside of Ion evolved, the dmabuf
emerged as the preferred standard for use in the kernel. This has made
the ion_handle an unnecessary abstraction and prone to race
conditions. ion_client is also now only used internally. We have enough
mechanisms for race conditions and leaks already so just drop ion_handle
and ion_client. This also includes ripping out most of the debugfs
infrastructure since much of that was tied to clients and handles.
The debugfs infrastructure was prone to give confusing data (orphaned
allocations) so it can be replaced with something better if people
actually want it.
Signed-off-by: Laura Abbott
---
drivers/staging/android/ion/ion-ioctl.c | 53 +--
drivers/staging/android/ion/ion.c | 697 ++--
drivers/staging/android/ion/ion.h | 73 +---
drivers/staging/android/uapi/ion.h | 25 +-
4 files changed, 49 insertions(+), 799 deletions(-)
diff --git a/drivers/staging/android/ion/ion-ioctl.c
b/drivers/staging/android/ion/ion-ioctl.c
index 4e7bf16..76427e4 100644
--- a/drivers/staging/android/ion/ion-ioctl.c
+++ b/drivers/staging/android/ion/ion-ioctl.c
@@ -21,9 +21,7 @@
#include "ion.h"
union ion_ioctl_arg {
- struct ion_fd_data fd;
struct ion_allocation_data allocation;
- struct ion_handle_data handle;
struct ion_heap_query query;
};
@@ -48,8 +46,6 @@ static int validate_ioctl_arg(unsigned int cmd, union
ion_ioctl_arg *arg)
static unsigned int ion_ioctl_dir(unsigned int cmd)
{
switch (cmd) {
- case ION_IOC_FREE:
- return _IOC_WRITE;
default:
return _IOC_DIR(cmd);
}
@@ -57,8 +53,6 @@ static unsigned int ion_ioctl_dir(unsigned int cmd)
long ion_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
{
- struct ion_client *client = filp->private_data;
- struct ion_handle *cleanup_handle = NULL;
int ret = 0;
unsigned int dir;
union ion_ioctl_arg data;
@@ -86,61 +80,28 @@ long ion_ioctl(struct file *filp, unsigned int cmd,
unsigned long arg)
switch (cmd) {
case ION_IOC_ALLOC:
{
- struct ion_handle *handle;
+ int fd;
- handle = ion_alloc(client, data.allocation.len,
+ fd = ion_alloc(data.allocation.len,
data.allocation.heap_id_mask,
data.allocation.flags);
- if (IS_ERR(handle))
- return PTR_ERR(handle);
+ if (fd < 0)
+ return fd;
- data.allocation.handle = handle->id;
+ data.allocation.fd = fd;
- cleanup_handle = handle;
- break;
- }
- case ION_IOC_FREE:
- {
- struct ion_handle *handle;
-
- mutex_lock(&client->lock);
- handle = ion_handle_get_by_id_nolock(client,
-data.handle.handle);
- if (IS_ERR(handle)) {
- mutex_unlock(&client->lock);
- return PTR_ERR(handle);
- }
- ion_free_nolock(client, handle);
- ion_handle_put_nolock(handle);
- mutex_unlock(&client->lock);
- break;
- }
- case ION_IOC_SHARE:
- {
- struct ion_handle *handle;
-
- handle = ion_handle_get_by_id(client, data.handle.handle);
- if (IS_ERR(handle))
- return PTR_ERR(handle);
- data.fd.fd = ion_share_dma_buf_fd(client, handle);
- ion_handle_put(handle);
- if (data.fd.fd < 0)
- ret = data.fd.fd;
break;
}
case ION_IOC_HEAP_QUERY:
- ret = ion_query_heaps(client, &data.query);
+ ret = ion_query_heaps(&data.query);
break;
default:
return -ENOTTY;
}
if (dir & _IOC_READ) {
- if (copy_to_user((void __user *)arg, &data, _IOC_SIZE(cmd))) {
- if (cleanup_handle)
- ion_free(client, cleanup_handle);
+ if (copy_to_user((void __user *)arg, &data, _IOC_SIZE(cmd)))
return -EFAULT;
- }
}
return ret;
}
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index 5a82bea..64c652b 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -90,7 +90,6 @@ static struct ion_buffer *ion_buffer_create(struct ion_heap
*heap,
buffer->heap = heap;
buffer->flags = flags;
- kref_init(&buffer->ref);
ret = heap->ops->allocate(heap,
[RFC PATCHv2 19/21] staging: android: ion: Drop ion_map_kernel interface
Nobody uses this interface externally. Drop it.
Signed-off-by: Laura Abbott
---
drivers/staging/android/ion/ion.c | 59 ---
1 file changed, 59 deletions(-)
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index 7d40233..5a82bea 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -424,22 +424,6 @@ static void *ion_buffer_kmap_get(struct ion_buffer *buffer)
return vaddr;
}
-static void *ion_handle_kmap_get(struct ion_handle *handle)
-{
- struct ion_buffer *buffer = handle->buffer;
- void *vaddr;
-
- if (handle->kmap_cnt) {
- handle->kmap_cnt++;
- return buffer->vaddr;
- }
- vaddr = ion_buffer_kmap_get(buffer);
- if (IS_ERR(vaddr))
- return vaddr;
- handle->kmap_cnt++;
- return vaddr;
-}
-
static void ion_buffer_kmap_put(struct ion_buffer *buffer)
{
buffer->kmap_cnt--;
@@ -462,49 +446,6 @@ static void ion_handle_kmap_put(struct ion_handle *handle)
ion_buffer_kmap_put(buffer);
}
-void *ion_map_kernel(struct ion_client *client, struct ion_handle *handle)
-{
- struct ion_buffer *buffer;
- void *vaddr;
-
- mutex_lock(&client->lock);
- if (!ion_handle_validate(client, handle)) {
- pr_err("%s: invalid handle passed to map_kernel.\n",
- __func__);
- mutex_unlock(&client->lock);
- return ERR_PTR(-EINVAL);
- }
-
- buffer = handle->buffer;
-
- if (!handle->buffer->heap->ops->map_kernel) {
- pr_err("%s: map_kernel is not implemented by this heap.\n",
- __func__);
- mutex_unlock(&client->lock);
- return ERR_PTR(-ENODEV);
- }
-
- mutex_lock(&buffer->lock);
- vaddr = ion_handle_kmap_get(handle);
- mutex_unlock(&buffer->lock);
- mutex_unlock(&client->lock);
- return vaddr;
-}
-EXPORT_SYMBOL(ion_map_kernel);
-
-void ion_unmap_kernel(struct ion_client *client, struct ion_handle *handle)
-{
- struct ion_buffer *buffer;
-
- mutex_lock(&client->lock);
- buffer = handle->buffer;
- mutex_lock(&buffer->lock);
- ion_handle_kmap_put(handle);
- mutex_unlock(&buffer->lock);
- mutex_unlock(&client->lock);
-}
-EXPORT_SYMBOL(ion_unmap_kernel);
-
static struct mutex debugfs_mutex;
static struct rb_root *ion_root_client;
static int is_client_alive(struct ion_client *client)
--
2.7.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC PATCHv2 17/21] staging: android: ion: Collapse internal header files
Ion current has ion_priv.h and ion.h as header files. ion.h was intended
to be used for public APIs but Ion never ended up really having anything
public. Combine the two headers so there is only one internal header.
Signed-off-by: Laura Abbott
---
drivers/staging/android/ion/ion-ioctl.c | 1 -
drivers/staging/android/ion/ion.c | 1 -
drivers/staging/android/ion/ion.h | 481
drivers/staging/android/ion/ion_carveout_heap.c | 1 -
drivers/staging/android/ion/ion_chunk_heap.c| 1 -
drivers/staging/android/ion/ion_cma_heap.c | 1 -
drivers/staging/android/ion/ion_heap.c | 1 -
drivers/staging/android/ion/ion_page_pool.c | 3 +-
drivers/staging/android/ion/ion_priv.h | 453 --
drivers/staging/android/ion/ion_system_heap.c | 1 -
10 files changed, 403 insertions(+), 541 deletions(-)
delete mode 100644 drivers/staging/android/ion/ion_priv.h
diff --git a/drivers/staging/android/ion/ion-ioctl.c
b/drivers/staging/android/ion/ion-ioctl.c
index 91b5c2b..4e7bf16 100644
--- a/drivers/staging/android/ion/ion-ioctl.c
+++ b/drivers/staging/android/ion/ion-ioctl.c
@@ -19,7 +19,6 @@
#include
#include "ion.h"
-#include "ion_priv.h"
union ion_ioctl_arg {
struct ion_fd_data fd;
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index fbab1e3..e1fb865 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -39,7 +39,6 @@
#include
#include "ion.h"
-#include "ion_priv.h"
bool ion_buffer_cached(struct ion_buffer *buffer)
{
diff --git a/drivers/staging/android/ion/ion.h
b/drivers/staging/android/ion/ion.h
index e8a6ffe..67fcb73 100644
--- a/drivers/staging/android/ion/ion.h
+++ b/drivers/staging/android/ion/ion.h
@@ -1,5 +1,5 @@
/*
- * drivers/staging/android/ion/ion.h
+ * drivers/staging/android/ion/ion_priv.h
*
* Copyright (C) 2011 Google, Inc.
*
@@ -14,24 +14,26 @@
*
*/
-#ifndef _LINUX_ION_H
-#define _LINUX_ION_H
+#ifndef _ION_PRIV_H
+#define _ION_PRIV_H
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
#include
+#include
#include "../uapi/ion.h"
-struct ion_handle;
-struct ion_device;
-struct ion_heap;
-struct ion_mapper;
-struct ion_client;
-struct ion_buffer;
-
/**
* struct ion_platform_heap - defines a heap in the given platform
* @type: type of the heap from ion_heap_type enum
- * @id:unique identifier for heap. When allocating higher
numbers
+ * @id:unique identifier for heap. When allocating higher
numb ers
* will be allocated from first. At allocation these are passed
* as a bit mask and therefore can not exceed ION_NUM_HEAP_IDS.
* @name: used for debug purposes
@@ -52,114 +54,433 @@ struct ion_platform_heap {
};
/**
- * struct ion_platform_data - array of platform heaps passed from board file
- * @nr:number of structures in the array
- * @heaps: array of platform_heap structions
+ * struct ion_buffer - metadata for a particular buffer
+ * @ref: reference count
+ * @node: node in the ion_device buffers tree
+ * @dev: back pointer to the ion_device
+ * @heap: back pointer to the heap the buffer came from
+ * @flags: buffer specific flags
+ * @private_flags: internal buffer specific flags
+ * @size: size of the buffer
+ * @priv_virt: private data to the buffer representable as
+ * a void *
+ * @lock: protects the buffers cnt fields
+ * @kmap_cnt: number of times the buffer is mapped to the kernel
+ * @vaddr: the kernel mapping if kmap_cnt is not zero
+ * @sg_table: the sg table for the buffer if dmap_cnt is not zero
+ * @pages: flat array of pages in the buffer -- used by fault
+ * handler and only valid for buffers that are faulted in
+ * @vmas: list of vma's mapping this buffer
+ * @handle_count: count of handles referencing this buffer
+ * @task_comm: taskcomm of last client to reference this buffer in a
+ * handle, used for debugging
+ * @pid: pid of last client to reference this buffer in a
+ * handle, used for debugging
+ */
+struct ion_buffer {
+ struct kref ref;
+ union {
+ struct rb_node node;
+ struct list_head list;
+ };
+ struct ion_device *dev;
+ struct ion_heap *heap;
+ unsigned long flags;
+ unsigned long private_flags;
+ size_t size;
+ void *priv_virt;
+ struct mutex lock;
+ int kmap_cnt;
+ void *vaddr;
+ struct sg_table *sg_table;
+ struct page **pages;
+ struct list_head vmas;
+ struct list_head attachments;
+ /* used to track or
[RFC PATCHv2 13/21] staging: android: ion: Use CMA APIs directly
When CMA was first introduced, its primary use was for DMA allocation
and the only way to get CMA memory was to call dma_alloc_coherent. This
put Ion in an awkward position since there was no device structure
readily available and setting one up messed up the coherency model.
These days, CMA can be allocated directly from the APIs. Switch to using
this model to avoid needing a dummy device. This also mitigates some of
the caching problems (e.g. dma_alloc_coherent only returning uncached
memory).
Signed-off-by: Laura Abbott
---
drivers/staging/android/ion/Kconfig| 7 +++
drivers/staging/android/ion/Makefile | 3 +-
drivers/staging/android/ion/ion_cma_heap.c | 97 --
3 files changed, 35 insertions(+), 72 deletions(-)
diff --git a/drivers/staging/android/ion/Kconfig
b/drivers/staging/android/ion/Kconfig
index 206c4de..15108c4 100644
--- a/drivers/staging/android/ion/Kconfig
+++ b/drivers/staging/android/ion/Kconfig
@@ -10,3 +10,10 @@ menuconfig ION
If you're not using Android its probably safe to
say N here.
+config ION_CMA_HEAP
+ bool "Ion CMA heap support"
+ depends on ION && CMA
+ help
+ Choose this option to enable CMA heaps with Ion. This heap is backed
+ by the Contiguous Memory Allocator (CMA). If your system has these
+ regions, you should say Y here.
diff --git a/drivers/staging/android/ion/Makefile
b/drivers/staging/android/ion/Makefile
index 26672a0..66d0c4a 100644
--- a/drivers/staging/android/ion/Makefile
+++ b/drivers/staging/android/ion/Makefile
@@ -1,6 +1,7 @@
obj-$(CONFIG_ION) += ion.o ion-ioctl.o ion_heap.o \
ion_page_pool.o ion_system_heap.o \
- ion_carveout_heap.o ion_chunk_heap.o ion_cma_heap.o
+ ion_carveout_heap.o ion_chunk_heap.o
+obj-$(CONFIG_ION_CMA_HEAP) += ion_cma_heap.o
ifdef CONFIG_COMPAT
obj-$(CONFIG_ION) += compat_ion.o
endif
diff --git a/drivers/staging/android/ion/ion_cma_heap.c
b/drivers/staging/android/ion/ion_cma_heap.c
index d562fd7..f3e0f59 100644
--- a/drivers/staging/android/ion/ion_cma_heap.c
+++ b/drivers/staging/android/ion/ion_cma_heap.c
@@ -19,24 +19,19 @@
#include
#include
#include
-#include
+#include
+#include
#include "ion.h"
#include "ion_priv.h"
struct ion_cma_heap {
struct ion_heap heap;
- struct device *dev;
+ struct cma *cma;
};
#define to_cma_heap(x) container_of(x, struct ion_cma_heap, heap)
-struct ion_cma_buffer_info {
- void *cpu_addr;
- dma_addr_t handle;
- struct sg_table *table;
-};
-
/* ION CMA heap operations functions */
static int ion_cma_allocate(struct ion_heap *heap, struct ion_buffer *buffer,
@@ -44,93 +39,53 @@ static int ion_cma_allocate(struct ion_heap *heap, struct
ion_buffer *buffer,
unsigned long flags)
{
struct ion_cma_heap *cma_heap = to_cma_heap(heap);
- struct device *dev = cma_heap->dev;
- struct ion_cma_buffer_info *info;
-
- dev_dbg(dev, "Request buffer allocation len %ld\n", len);
-
- if (buffer->flags & ION_FLAG_CACHED)
- return -EINVAL;
+ struct sg_table *table;
+ struct page *pages;
+ int ret;
- info = kzalloc(sizeof(*info), GFP_KERNEL);
- if (!info)
+ pages = cma_alloc(cma_heap->cma, len, 0, GFP_KERNEL);
+ if (!pages)
return -ENOMEM;
- info->cpu_addr = dma_alloc_coherent(dev, len, &(info->handle),
- GFP_HIGHUSER | __GFP_ZERO);
-
- if (!info->cpu_addr) {
- dev_err(dev, "Fail to allocate buffer\n");
+ table = kmalloc(sizeof(struct sg_table), GFP_KERNEL);
+ if (!table)
goto err;
- }
- info->table = kmalloc(sizeof(*info->table), GFP_KERNEL);
- if (!info->table)
+ ret = sg_alloc_table(table, 1, GFP_KERNEL);
+ if (ret)
goto free_mem;
- if (dma_get_sgtable(dev, info->table, info->cpu_addr, info->handle,
- len))
- goto free_table;
- /* keep this for memory release */
- buffer->priv_virt = info;
- buffer->sg_table = info->table;
- dev_dbg(dev, "Allocate buffer %p\n", buffer);
+ sg_set_page(table->sgl, pages, len, 0);
+
+ buffer->priv_virt = pages;
+ buffer->sg_table = table;
return 0;
-free_table:
- kfree(info->table);
free_mem:
- dma_free_coherent(dev, len, info->cpu_addr, info->handle);
+ kfree(table);
err:
- kfree(info);
+ cma_release(cma_heap->cma, pages, buffer->size);
return -ENOMEM;
}
static void ion_cma_free(struct ion_buffer *buffer)
{
struct ion_cma_heap *cma_heap = to_cma_heap(buffer->heap);
- struct device *dev = cma_heap->dev;
- struct ion_cma_buffer_info *info = buffer->priv_virt;
+ struct page *pages = buffer->priv_virt
[RFC PATCHv2 12/21] staging: android: ion: Remove old platform support
Device specific platform support has been haphazard for Ion. There have been several independent attempts and there are still objections to what bindings exist right now. Just remove everything for a fresh start. Signed-off-by: Laura Abbott --- drivers/staging/android/ion/Kconfig| 42 --- drivers/staging/android/ion/Makefile | 7 - drivers/staging/android/ion/hisilicon/Kconfig | 5 - drivers/staging/android/ion/hisilicon/Makefile | 1 - drivers/staging/android/ion/hisilicon/hi6220_ion.c | 113 drivers/staging/android/ion/ion_dummy_driver.c | 156 --- drivers/staging/android/ion/ion_of.c | 184 - drivers/staging/android/ion/ion_of.h | 37 --- drivers/staging/android/ion/ion_test.c | 305 - drivers/staging/android/ion/tegra/Makefile | 1 - drivers/staging/android/ion/tegra/tegra_ion.c | 80 -- drivers/staging/android/uapi/ion_test.h| 69 - 12 files changed, 1000 deletions(-) delete mode 100644 drivers/staging/android/ion/hisilicon/Kconfig delete mode 100644 drivers/staging/android/ion/hisilicon/Makefile delete mode 100644 drivers/staging/android/ion/hisilicon/hi6220_ion.c delete mode 100644 drivers/staging/android/ion/ion_dummy_driver.c delete mode 100644 drivers/staging/android/ion/ion_of.c delete mode 100644 drivers/staging/android/ion/ion_of.h delete mode 100644 drivers/staging/android/ion/ion_test.c delete mode 100644 drivers/staging/android/ion/tegra/Makefile delete mode 100644 drivers/staging/android/ion/tegra/tegra_ion.c delete mode 100644 drivers/staging/android/uapi/ion_test.h diff --git a/drivers/staging/android/ion/Kconfig b/drivers/staging/android/ion/Kconfig index c8fb413..206c4de 100644 --- a/drivers/staging/android/ion/Kconfig +++ b/drivers/staging/android/ion/Kconfig @@ -10,45 +10,3 @@ menuconfig ION If you're not using Android its probably safe to say N here. -config ION_TEST - tristate "Ion Test Device" - depends on ION - help - Choose this option to create a device that can be used to test the - kernel and device side ION functions. - -config ION_DUMMY - bool "Dummy Ion driver" - depends on ION - help - Provides a dummy ION driver that registers the - /dev/ion device and some basic heaps. This can - be used for testing the ION infrastructure if - one doesn't have access to hardware drivers that - use ION. - -config ION_TEGRA - tristate "Ion for Tegra" - depends on ARCH_TEGRA && ION - help - Choose this option if you wish to use ion on an nVidia Tegra. - -config ION_HISI - tristate "Ion for Hisilicon" - depends on ARCH_HISI && ION - select ION_OF - help - Choose this option if you wish to use ion on Hisilicon Platform. - -source "drivers/staging/android/ion/hisilicon/Kconfig" - -config ION_OF - bool "Devicetree support for Ion" - depends on ION && OF_ADDRESS - help - Provides base support for defining Ion heaps in devicetree - and setting them up. Also includes functions for platforms - to parse the devicetree and expand for their own custom - extensions - - If using Ion and devicetree, you should say Y here diff --git a/drivers/staging/android/ion/Makefile b/drivers/staging/android/ion/Makefile index 5d630a0..26672a0 100644 --- a/drivers/staging/android/ion/Makefile +++ b/drivers/staging/android/ion/Makefile @@ -1,13 +1,6 @@ obj-$(CONFIG_ION) += ion.o ion-ioctl.o ion_heap.o \ ion_page_pool.o ion_system_heap.o \ ion_carveout_heap.o ion_chunk_heap.o ion_cma_heap.o -obj-$(CONFIG_ION_TEST) += ion_test.o ifdef CONFIG_COMPAT obj-$(CONFIG_ION) += compat_ion.o endif - -obj-$(CONFIG_ION_DUMMY) += ion_dummy_driver.o -obj-$(CONFIG_ION_TEGRA) += tegra/ -obj-$(CONFIG_ION_HISI) += hisilicon/ -obj-$(CONFIG_ION_OF) += ion_of.o - diff --git a/drivers/staging/android/ion/hisilicon/Kconfig b/drivers/staging/android/ion/hisilicon/Kconfig deleted file mode 100644 index 2b4bd07..000 --- a/drivers/staging/android/ion/hisilicon/Kconfig +++ /dev/null @@ -1,5 +0,0 @@ -config HI6220_ION -bool "Hi6220 ION Driver" -depends on ARCH_HISI && ION -help - Build the Hisilicon Hi6220 ion driver. diff --git a/drivers/staging/android/ion/hisilicon/Makefile b/drivers/staging/android/ion/hisilicon/Makefile deleted file mode 100644 index 2a89414..000 --- a/drivers/staging/android/ion/hisilicon/Makefile +++ /dev/null @@ -1 +0,0 @@ -obj-$(CONFIG_HI6220_ION) += hi6220_ion.o diff --git a/drivers/staging/android/ion/hisilicon/hi6220_ion.c b/drivers/staging/android/ion/hisilicon/hi6220_ion.c deleted file mode 100644 index 0de7897..000 --- a/drivers/staging/android/ion/hisilicon/hi6220_ion.c +++ /dev/null @@ -1,113 +0,0
[RFC PATCHv2 15/21] staging: android: ion: Break the ABI in the name of forward progress
To: Sumit Semwal
To: Riley Andrews
Cc: rom...@google.com
To: a...@android.com
To: Riley Andrews
Cc: de...@driverdev.osuosl.org
Cc: linux-ker...@vger.kernel.org
Cc: linaro-mm-...@lists.linaro.org
Cc: Greg Kroah-Hartman
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux-me...@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: Brian Starkey
Cc: Daniel Vetter
Cc: Mark Brown
Cc: Benjamin Gaignard
Cc: linux...@kvack.org
Cc: Laurent Pinchart
Several of the Ion ioctls were designed in such a way that they
necessitate compat ioctls. We're breaking a bunch of other ABIs and
cleaning stuff up anyway so let's follow the ioctl guidelines and clean
things up while everyone is busy converting things over anyway. As part
of this, also remove the useless alignment field from the allocation
structure.
Signed-off-by: Laura Abbott
---
drivers/staging/android/ion/Makefile | 3 -
drivers/staging/android/ion/compat_ion.c | 152 ---
drivers/staging/android/ion/compat_ion.h | 29 --
drivers/staging/android/ion/ion-ioctl.c | 1 -
drivers/staging/android/ion/ion.c| 5 +-
drivers/staging/android/uapi/ion.h | 19 ++--
6 files changed, 11 insertions(+), 198 deletions(-)
delete mode 100644 drivers/staging/android/ion/compat_ion.c
delete mode 100644 drivers/staging/android/ion/compat_ion.h
diff --git a/drivers/staging/android/ion/Makefile
b/drivers/staging/android/ion/Makefile
index 66d0c4a..a892afa 100644
--- a/drivers/staging/android/ion/Makefile
+++ b/drivers/staging/android/ion/Makefile
@@ -2,6 +2,3 @@ obj-$(CONFIG_ION) +=ion.o ion-ioctl.o ion_heap.o \
ion_page_pool.o ion_system_heap.o \
ion_carveout_heap.o ion_chunk_heap.o
obj-$(CONFIG_ION_CMA_HEAP) += ion_cma_heap.o
-ifdef CONFIG_COMPAT
-obj-$(CONFIG_ION) += compat_ion.o
-endif
diff --git a/drivers/staging/android/ion/compat_ion.c
b/drivers/staging/android/ion/compat_ion.c
deleted file mode 100644
index 5037ddd..000
--- a/drivers/staging/android/ion/compat_ion.c
+++ /dev/null
@@ -1,152 +0,0 @@
-/*
- * drivers/staging/android/ion/compat_ion.c
- *
- * Copyright (C) 2013 Google, Inc.
- *
- * This software is licensed under the terms of the GNU General Public
- * License version 2, as published by the Free Software Foundation, and
- * may be copied, distributed, and modified under those terms.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
- */
-
-#include
-#include
-#include
-
-#include "ion.h"
-#include "compat_ion.h"
-
-/* See drivers/staging/android/uapi/ion.h for the definition of these structs
*/
-struct compat_ion_allocation_data {
- compat_size_t len;
- compat_size_t align;
- compat_uint_t heap_id_mask;
- compat_uint_t flags;
- compat_int_t handle;
-};
-
-struct compat_ion_handle_data {
- compat_int_t handle;
-};
-
-#define COMPAT_ION_IOC_ALLOC _IOWR(ION_IOC_MAGIC, 0, \
- struct compat_ion_allocation_data)
-#define COMPAT_ION_IOC_FREE_IOWR(ION_IOC_MAGIC, 1, \
- struct compat_ion_handle_data)
-
-static int compat_get_ion_allocation_data(
- struct compat_ion_allocation_data __user *data32,
- struct ion_allocation_data __user *data)
-{
- compat_size_t s;
- compat_uint_t u;
- compat_int_t i;
- int err;
-
- err = get_user(s, &data32->len);
- err |= put_user(s, &data->len);
- err |= get_user(s, &data32->align);
- err |= put_user(s, &data->align);
- err |= get_user(u, &data32->heap_id_mask);
- err |= put_user(u, &data->heap_id_mask);
- err |= get_user(u, &data32->flags);
- err |= put_user(u, &data->flags);
- err |= get_user(i, &data32->handle);
- err |= put_user(i, &data->handle);
-
- return err;
-}
-
-static int compat_get_ion_handle_data(
- struct compat_ion_handle_data __user *data32,
- struct ion_handle_data __user *data)
-{
- compat_int_t i;
- int err;
-
- err = get_user(i, &data32->handle);
- err |= put_user(i, &data->handle);
-
- return err;
-}
-
-static int compat_put_ion_allocation_data(
- struct compat_ion_allocation_data __user *data32,
- struct ion_allocation_data __user *data)
-{
- compat_size_t s;
- compat_uint_t u;
- compat_int_t i;
- int err;
-
- err = get_user(s, &data->len);
- err |= put_user(s, &data32->len);
- err |= get_user(s, &data->align);
- err |= put_user(s, &data32->align);
- err |= get_user(u, &data->heap_id_mask);
- err |= put_user(u, &data32->heap_id_mask);
- err |= get_user(u, &data
[RFC PATCHv2 11/21] staging: android: ion: Remove duplicate ION_IOC_MAP
ION_IOC_MAP is the same as ION_IOC_SHARE. We really don't need two
identical interfaces. Remove it.
Signed-off-by: Laura Abbott
---
drivers/staging/android/ion/compat_ion.c | 1 -
drivers/staging/android/ion/ion-ioctl.c | 1 -
drivers/staging/android/uapi/ion.h | 10 --
3 files changed, 12 deletions(-)
diff --git a/drivers/staging/android/ion/compat_ion.c
b/drivers/staging/android/ion/compat_ion.c
index ae1ffc3..5037ddd 100644
--- a/drivers/staging/android/ion/compat_ion.c
+++ b/drivers/staging/android/ion/compat_ion.c
@@ -144,7 +144,6 @@ long compat_ion_ioctl(struct file *filp, unsigned int cmd,
unsigned long arg)
(unsigned long)data);
}
case ION_IOC_SHARE:
- case ION_IOC_MAP:
return filp->f_op->unlocked_ioctl(filp, cmd,
(unsigned long)compat_ptr(arg));
default:
diff --git a/drivers/staging/android/ion/ion-ioctl.c
b/drivers/staging/android/ion/ion-ioctl.c
index 7b54eea..a361724 100644
--- a/drivers/staging/android/ion/ion-ioctl.c
+++ b/drivers/staging/android/ion/ion-ioctl.c
@@ -118,7 +118,6 @@ long ion_ioctl(struct file *filp, unsigned int cmd,
unsigned long arg)
break;
}
case ION_IOC_SHARE:
- case ION_IOC_MAP:
{
struct ion_handle *handle;
diff --git a/drivers/staging/android/uapi/ion.h
b/drivers/staging/android/uapi/ion.h
index 3a59044..abd72fd 100644
--- a/drivers/staging/android/uapi/ion.h
+++ b/drivers/staging/android/uapi/ion.h
@@ -164,16 +164,6 @@ struct ion_heap_query {
#define ION_IOC_FREE _IOWR(ION_IOC_MAGIC, 1, struct ion_handle_data)
/**
- * DOC: ION_IOC_MAP - get a file descriptor to mmap
- *
- * Takes an ion_fd_data struct with the handle field populated with a valid
- * opaque handle. Returns the struct with the fd field set to a file
- * descriptor open in the current address space. This file descriptor
- * can then be used as an argument to mmap.
- */
-#define ION_IOC_MAP_IOWR(ION_IOC_MAGIC, 2, struct ion_fd_data)
-
-/**
* DOC: ION_IOC_SHARE - creates a file descriptor to use to share an allocation
*
* Takes an ion_fd_data struct with the handle field populated with a valid
--
2.7.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC PATCHv2 16/21] staging: android: ion: Get rid of ion_phys_addr_t
Once upon a time, phys_addr_t was not everywhere in the kernel. These
days it is used enough places that having a separate Ion type doesn't
make sense. Remove the extra type and just use phys_addr_t directly.
Signed-off-by: Laura Abbott
---
drivers/staging/android/ion/ion.h | 12 ++--
drivers/staging/android/ion/ion_carveout_heap.c | 10 +-
drivers/staging/android/ion/ion_chunk_heap.c| 6 +++---
drivers/staging/android/ion/ion_heap.c | 4 ++--
4 files changed, 12 insertions(+), 20 deletions(-)
diff --git a/drivers/staging/android/ion/ion.h
b/drivers/staging/android/ion/ion.h
index 3b4bff5..e8a6ffe 100644
--- a/drivers/staging/android/ion/ion.h
+++ b/drivers/staging/android/ion/ion.h
@@ -28,14 +28,6 @@ struct ion_mapper;
struct ion_client;
struct ion_buffer;
-/*
- * This should be removed some day when phys_addr_t's are fully
- * plumbed in the kernel, and all instances of ion_phys_addr_t should
- * be converted to phys_addr_t. For the time being many kernel interfaces
- * do not accept phys_addr_t's that would have to
- */
-#define ion_phys_addr_t unsigned long
-
/**
* struct ion_platform_heap - defines a heap in the given platform
* @type: type of the heap from ion_heap_type enum
@@ -53,9 +45,9 @@ struct ion_platform_heap {
enum ion_heap_type type;
unsigned int id;
const char *name;
- ion_phys_addr_t base;
+ phys_addr_t base;
size_t size;
- ion_phys_addr_t align;
+ phys_addr_t align;
void *priv;
};
diff --git a/drivers/staging/android/ion/ion_carveout_heap.c
b/drivers/staging/android/ion/ion_carveout_heap.c
index e0e360f..1419a89 100644
--- a/drivers/staging/android/ion/ion_carveout_heap.c
+++ b/drivers/staging/android/ion/ion_carveout_heap.c
@@ -30,10 +30,10 @@
struct ion_carveout_heap {
struct ion_heap heap;
struct gen_pool *pool;
- ion_phys_addr_t base;
+ phys_addr_t base;
};
-static ion_phys_addr_t ion_carveout_allocate(struct ion_heap *heap,
+static phys_addr_t ion_carveout_allocate(struct ion_heap *heap,
unsigned long size)
{
struct ion_carveout_heap *carveout_heap =
@@ -46,7 +46,7 @@ static ion_phys_addr_t ion_carveout_allocate(struct ion_heap
*heap,
return offset;
}
-static void ion_carveout_free(struct ion_heap *heap, ion_phys_addr_t addr,
+static void ion_carveout_free(struct ion_heap *heap, phys_addr_t addr,
unsigned long size)
{
struct ion_carveout_heap *carveout_heap =
@@ -63,7 +63,7 @@ static int ion_carveout_heap_allocate(struct ion_heap *heap,
unsigned long flags)
{
struct sg_table *table;
- ion_phys_addr_t paddr;
+ phys_addr_t paddr;
int ret;
table = kmalloc(sizeof(*table), GFP_KERNEL);
@@ -96,7 +96,7 @@ static void ion_carveout_heap_free(struct ion_buffer *buffer)
struct ion_heap *heap = buffer->heap;
struct sg_table *table = buffer->sg_table;
struct page *page = sg_page(table->sgl);
- ion_phys_addr_t paddr = PFN_PHYS(page_to_pfn(page));
+ phys_addr_t paddr = PFN_PHYS(page_to_pfn(page));
ion_heap_buffer_zero(buffer);
diff --git a/drivers/staging/android/ion/ion_chunk_heap.c
b/drivers/staging/android/ion/ion_chunk_heap.c
index 46e13f6..606f25f 100644
--- a/drivers/staging/android/ion/ion_chunk_heap.c
+++ b/drivers/staging/android/ion/ion_chunk_heap.c
@@ -27,7 +27,7 @@
struct ion_chunk_heap {
struct ion_heap heap;
struct gen_pool *pool;
- ion_phys_addr_t base;
+ phys_addr_t base;
unsigned long chunk_size;
unsigned long size;
unsigned long allocated;
@@ -151,8 +151,8 @@ struct ion_heap *ion_chunk_heap_create(struct
ion_platform_heap *heap_data)
chunk_heap->heap.ops = &chunk_heap_ops;
chunk_heap->heap.type = ION_HEAP_TYPE_CHUNK;
chunk_heap->heap.flags = ION_HEAP_FLAG_DEFER_FREE;
- pr_debug("%s: base %lu size %zu \n", __func__,
-chunk_heap->base, heap_data->size);
+ pr_debug("%s: base %pa size %zu \n", __func__,
+&chunk_heap->base, heap_data->size);
return &chunk_heap->heap;
diff --git a/drivers/staging/android/ion/ion_heap.c
b/drivers/staging/android/ion/ion_heap.c
index c69d0bd..03b554f 100644
--- a/drivers/staging/android/ion/ion_heap.c
+++ b/drivers/staging/android/ion/ion_heap.c
@@ -343,9 +343,9 @@ struct ion_heap *ion_heap_create(struct ion_platform_heap
*heap_data)
}
if (IS_ERR_OR_NULL(heap)) {
- pr_err("%s: error creating heap %s type %d base %lu size %zu\n",
+ pr_err("%s: error creating heap %s type %d base %pa size %zu\n",
__func__, heap_data->name, heap_data->type,
- heap_data->base, heap_data->size);
+ &heap_data->base, heap_data->size);
[RFC PATCHv2 14/21] staging: android: ion: Stop butchering the DMA address
Now that we have proper caching, stop setting the DMA address manually.
It should be set after properly calling dma_map.
Signed-off-by: Laura Abbott
---
drivers/staging/android/ion/ion.c | 17 +
1 file changed, 1 insertion(+), 16 deletions(-)
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index 3d979ef5..65638f5 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -81,8 +81,7 @@ static struct ion_buffer *ion_buffer_create(struct ion_heap
*heap,
{
struct ion_buffer *buffer;
struct sg_table *table;
- struct scatterlist *sg;
- int i, ret;
+ int ret;
buffer = kzalloc(sizeof(*buffer), GFP_KERNEL);
if (!buffer)
@@ -119,20 +118,6 @@ static struct ion_buffer *ion_buffer_create(struct
ion_heap *heap,
INIT_LIST_HEAD(&buffer->vmas);
INIT_LIST_HEAD(&buffer->attachments);
mutex_init(&buffer->lock);
- /*
-* this will set up dma addresses for the sglist -- it is not
-* technically correct as per the dma api -- a specific
-* device isn't really taking ownership here. However, in practice on
-* our systems the only dma_address space is physical addresses.
-* Additionally, we can't afford the overhead of invalidating every
-* allocation via dma_map_sg. The implicit contract here is that
-* memory coming from the heaps is ready for dma, ie if it has a
-* cached mapping that mapping has been invalidated
-*/
- for_each_sg(buffer->sg_table->sgl, sg, buffer->sg_table->nents, i) {
- sg_dma_address(sg) = sg_phys(sg);
- sg_dma_len(sg) = sg->length;
- }
mutex_lock(&dev->buffer_lock);
ion_buffer_add(dev, buffer);
mutex_unlock(&dev->buffer_lock);
--
2.7.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC PATCHv2 01/21] cma: Store a name in the cma structure
Frameworks that may want to enumerate CMA heaps (e.g. Ion) will find it
useful to have an explicit name attached to each region. Store the name
in each CMA structure.
Signed-off-by: Laura Abbott
---
drivers/base/dma-contiguous.c | 5 +++--
include/linux/cma.h | 4 +++-
mm/cma.c | 11 +--
mm/cma.h | 1 +
mm/cma_debug.c| 2 +-
5 files changed, 17 insertions(+), 6 deletions(-)
diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
index b55804c..ea9726e 100644
--- a/drivers/base/dma-contiguous.c
+++ b/drivers/base/dma-contiguous.c
@@ -165,7 +165,8 @@ int __init dma_contiguous_reserve_area(phys_addr_t size,
phys_addr_t base,
{
int ret;
- ret = cma_declare_contiguous(base, size, limit, 0, 0, fixed, res_cma);
+ ret = cma_declare_contiguous(base, size, limit, 0, 0, fixed,
+ "reserved", res_cma);
if (ret)
return ret;
@@ -258,7 +259,7 @@ static int __init rmem_cma_setup(struct reserved_mem *rmem)
return -EINVAL;
}
- err = cma_init_reserved_mem(rmem->base, rmem->size, 0, &cma);
+ err = cma_init_reserved_mem(rmem->base, rmem->size, 0, rmem->name,
&cma);
if (err) {
pr_err("Reserved memory: unable to setup CMA region\n");
return err;
diff --git a/include/linux/cma.h b/include/linux/cma.h
index 03f32d0..d41d1f8 100644
--- a/include/linux/cma.h
+++ b/include/linux/cma.h
@@ -21,13 +21,15 @@ struct cma;
extern unsigned long totalcma_pages;
extern phys_addr_t cma_get_base(const struct cma *cma);
extern unsigned long cma_get_size(const struct cma *cma);
+extern const char *cma_get_name(const struct cma *cma);
extern int __init cma_declare_contiguous(phys_addr_t base,
phys_addr_t size, phys_addr_t limit,
phys_addr_t alignment, unsigned int order_per_bit,
- bool fixed, struct cma **res_cma);
+ bool fixed, const char *name, struct cma **res_cma);
extern int cma_init_reserved_mem(phys_addr_t base, phys_addr_t size,
unsigned int order_per_bit,
+ const char *name,
struct cma **res_cma);
extern struct page *cma_alloc(struct cma *cma, size_t count, unsigned int
align,
gfp_t gfp_mask);
diff --git a/mm/cma.c b/mm/cma.c
index a6033e3..0d187b1 100644
--- a/mm/cma.c
+++ b/mm/cma.c
@@ -53,6 +53,11 @@ unsigned long cma_get_size(const struct cma *cma)
return cma->count << PAGE_SHIFT;
}
+const char *cma_get_name(const struct cma *cma)
+{
+ return cma->name ? cma->name : "(undefined)";
+}
+
static unsigned long cma_bitmap_aligned_mask(const struct cma *cma,
int align_order)
{
@@ -168,6 +173,7 @@ core_initcall(cma_init_reserved_areas);
*/
int __init cma_init_reserved_mem(phys_addr_t base, phys_addr_t size,
unsigned int order_per_bit,
+const char *name,
struct cma **res_cma)
{
struct cma *cma;
@@ -201,6 +207,7 @@ int __init cma_init_reserved_mem(phys_addr_t base,
phys_addr_t size,
cma->base_pfn = PFN_DOWN(base);
cma->count = size >> PAGE_SHIFT;
cma->order_per_bit = order_per_bit;
+ cma->name = name;
*res_cma = cma;
cma_area_count++;
totalcma_pages += (size / PAGE_SIZE);
@@ -229,7 +236,7 @@ int __init cma_init_reserved_mem(phys_addr_t base,
phys_addr_t size,
int __init cma_declare_contiguous(phys_addr_t base,
phys_addr_t size, phys_addr_t limit,
phys_addr_t alignment, unsigned int order_per_bit,
- bool fixed, struct cma **res_cma)
+ bool fixed, const char *name, struct cma **res_cma)
{
phys_addr_t memblock_end = memblock_end_of_DRAM();
phys_addr_t highmem_start;
@@ -335,7 +342,7 @@ int __init cma_declare_contiguous(phys_addr_t base,
base = addr;
}
- ret = cma_init_reserved_mem(base, size, order_per_bit, res_cma);
+ ret = cma_init_reserved_mem(base, size, order_per_bit, name, res_cma);
if (ret)
goto err;
diff --git a/mm/cma.h b/mm/cma.h
index 17c75a4..4986128 100644
--- a/mm/cma.h
+++ b/mm/cma.h
@@ -11,6 +11,7 @@ struct cma {
struct hlist_head mem_head;
spinlock_t mem_head_lock;
#endif
+ const char *name;
};
extern struct cma cma_areas[MAX_CMA_AREAS];
diff --git a/mm/cma_debug.c b/mm/cma_debug.c
index ffc0c3d..595b757 100644
--- a/mm/cma_debug.c
+++ b/mm/cma_debug.c
@@ -167,7 +167,7 @@ static void cma_debugfs_add_one(struct cma *cma, int idx)
char name[16];
int u32s;
- sprintf(name, "cma-%
[RFC PATCHv2 04/21] staging: android: ion: Remove alignment from allocation field
The align field was supposed to be used to specify the alignment of
the allocation. Nobody actually does anything with it except to check
if the alignment specified is out of bounds. Since this has no effect
on the actual allocation, just remove it.
Signed-off-by: Laura Abbott
---
drivers/staging/android/ion/ion-ioctl.c | 1 -
drivers/staging/android/ion/ion.c | 14 ++
drivers/staging/android/ion/ion.h | 5 +
drivers/staging/android/ion/ion_carveout_heap.c | 10 +++---
drivers/staging/android/ion/ion_chunk_heap.c| 9 +++--
drivers/staging/android/ion/ion_cma_heap.c | 5 +
drivers/staging/android/ion/ion_priv.h | 2 +-
drivers/staging/android/ion/ion_system_heap.c | 9 +
8 files changed, 16 insertions(+), 39 deletions(-)
diff --git a/drivers/staging/android/ion/ion-ioctl.c
b/drivers/staging/android/ion/ion-ioctl.c
index 9ff815a..5b2e93f 100644
--- a/drivers/staging/android/ion/ion-ioctl.c
+++ b/drivers/staging/android/ion/ion-ioctl.c
@@ -95,7 +95,6 @@ long ion_ioctl(struct file *filp, unsigned int cmd, unsigned
long arg)
struct ion_handle *handle;
handle = ion_alloc(client, data.allocation.len,
- data.allocation.align,
data.allocation.heap_id_mask,
data.allocation.flags);
if (IS_ERR(handle))
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index f45115f..c2adfe1 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -103,7 +103,6 @@ static void ion_buffer_add(struct ion_device *dev,
static struct ion_buffer *ion_buffer_create(struct ion_heap *heap,
struct ion_device *dev,
unsigned long len,
- unsigned long align,
unsigned long flags)
{
struct ion_buffer *buffer;
@@ -119,15 +118,14 @@ static struct ion_buffer *ion_buffer_create(struct
ion_heap *heap,
buffer->flags = flags;
kref_init(&buffer->ref);
- ret = heap->ops->allocate(heap, buffer, len, align, flags);
+ ret = heap->ops->allocate(heap, buffer, len, flags);
if (ret) {
if (!(heap->flags & ION_HEAP_FLAG_DEFER_FREE))
goto err2;
ion_heap_freelist_drain(heap, 0);
- ret = heap->ops->allocate(heap, buffer, len, align,
- flags);
+ ret = heap->ops->allocate(heap, buffer, len, flags);
if (ret)
goto err2;
}
@@ -401,7 +399,7 @@ static int ion_handle_add(struct ion_client *client, struct
ion_handle *handle)
}
struct ion_handle *ion_alloc(struct ion_client *client, size_t len,
-size_t align, unsigned int heap_id_mask,
+unsigned int heap_id_mask,
unsigned int flags)
{
struct ion_handle *handle;
@@ -410,8 +408,8 @@ struct ion_handle *ion_alloc(struct ion_client *client,
size_t len,
struct ion_heap *heap;
int ret;
- pr_debug("%s: len %zu align %zu heap_id_mask %u flags %x\n", __func__,
-len, align, heap_id_mask, flags);
+ pr_debug("%s: len %zu heap_id_mask %u flags %x\n", __func__,
+len, heap_id_mask, flags);
/*
* traverse the list of heaps available in this system in priority
* order. If the heap type is supported by the client, and matches the
@@ -428,7 +426,7 @@ struct ion_handle *ion_alloc(struct ion_client *client,
size_t len,
/* if the caller didn't specify this heap id */
if (!((1 << heap->id) & heap_id_mask))
continue;
- buffer = ion_buffer_create(heap, dev, len, align, flags);
+ buffer = ion_buffer_create(heap, dev, len, flags);
if (!IS_ERR(buffer))
break;
}
diff --git a/drivers/staging/android/ion/ion.h
b/drivers/staging/android/ion/ion.h
index 93dafb4..3b4bff5 100644
--- a/drivers/staging/android/ion/ion.h
+++ b/drivers/staging/android/ion/ion.h
@@ -45,7 +45,6 @@ struct ion_buffer;
* @name: used for debug purposes
* @base: base address of heap in physical memory if applicable
* @size: size of the heap in bytes if applicable
- * @align: required alignment in physical memory if applicable
* @priv: private info passed from the board file
*
* Provided by the board file.
@@ -93,8 +92,6 @@ void ion_client_destroy(struct ion_client *client);
* ion_alloc - allocate ion memory
* @client:the client
* @len: size of the allocation
- * @
[RFC PATCHv2 10/21] staging: android: ion: Remove import interface
With the expansion of dma-buf and the move for Ion to be come just an
allocator, the import mechanism is mostly useless. There isn't a kernel
component to Ion anymore and handles are private to Ion. Remove this
interface.
Signed-off-by: Laura Abbott
---
drivers/staging/android/ion/compat_ion.c | 1 -
drivers/staging/android/ion/ion-ioctl.c | 11 -
drivers/staging/android/ion/ion.c| 76
drivers/staging/android/uapi/ion.h | 9
4 files changed, 97 deletions(-)
diff --git a/drivers/staging/android/ion/compat_ion.c
b/drivers/staging/android/ion/compat_ion.c
index 5b192ea..ae1ffc3 100644
--- a/drivers/staging/android/ion/compat_ion.c
+++ b/drivers/staging/android/ion/compat_ion.c
@@ -145,7 +145,6 @@ long compat_ion_ioctl(struct file *filp, unsigned int cmd,
unsigned long arg)
}
case ION_IOC_SHARE:
case ION_IOC_MAP:
- case ION_IOC_IMPORT:
return filp->f_op->unlocked_ioctl(filp, cmd,
(unsigned long)compat_ptr(arg));
default:
diff --git a/drivers/staging/android/ion/ion-ioctl.c
b/drivers/staging/android/ion/ion-ioctl.c
index 2b475bf..7b54eea 100644
--- a/drivers/staging/android/ion/ion-ioctl.c
+++ b/drivers/staging/android/ion/ion-ioctl.c
@@ -131,17 +131,6 @@ long ion_ioctl(struct file *filp, unsigned int cmd,
unsigned long arg)
ret = data.fd.fd;
break;
}
- case ION_IOC_IMPORT:
- {
- struct ion_handle *handle;
-
- handle = ion_import_dma_buf_fd(client, data.fd.fd);
- if (IS_ERR(handle))
- ret = PTR_ERR(handle);
- else
- data.handle.handle = handle->id;
- break;
- }
case ION_IOC_HEAP_QUERY:
ret = ion_query_heaps(client, &data.query);
break;
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index 125c537..3d979ef5 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -274,24 +274,6 @@ int ion_handle_put(struct ion_handle *handle)
return ret;
}
-static struct ion_handle *ion_handle_lookup(struct ion_client *client,
- struct ion_buffer *buffer)
-{
- struct rb_node *n = client->handles.rb_node;
-
- while (n) {
- struct ion_handle *entry = rb_entry(n, struct ion_handle, node);
-
- if (buffer < entry->buffer)
- n = n->rb_left;
- else if (buffer > entry->buffer)
- n = n->rb_right;
- else
- return entry;
- }
- return ERR_PTR(-EINVAL);
-}
-
struct ion_handle *ion_handle_get_by_id_nolock(struct ion_client *client,
int id)
{
@@ -1023,64 +1005,6 @@ int ion_share_dma_buf_fd(struct ion_client *client,
struct ion_handle *handle)
}
EXPORT_SYMBOL(ion_share_dma_buf_fd);
-struct ion_handle *ion_import_dma_buf(struct ion_client *client,
- struct dma_buf *dmabuf)
-{
- struct ion_buffer *buffer;
- struct ion_handle *handle;
- int ret;
-
- /* if this memory came from ion */
-
- if (dmabuf->ops != &dma_buf_ops) {
- pr_err("%s: can not import dmabuf from another exporter\n",
- __func__);
- return ERR_PTR(-EINVAL);
- }
- buffer = dmabuf->priv;
-
- mutex_lock(&client->lock);
- /* if a handle exists for this buffer just take a reference to it */
- handle = ion_handle_lookup(client, buffer);
- if (!IS_ERR(handle)) {
- ion_handle_get(handle);
- mutex_unlock(&client->lock);
- goto end;
- }
-
- handle = ion_handle_create(client, buffer);
- if (IS_ERR(handle)) {
- mutex_unlock(&client->lock);
- goto end;
- }
-
- ret = ion_handle_add(client, handle);
- mutex_unlock(&client->lock);
- if (ret) {
- ion_handle_put(handle);
- handle = ERR_PTR(ret);
- }
-
-end:
- return handle;
-}
-EXPORT_SYMBOL(ion_import_dma_buf);
-
-struct ion_handle *ion_import_dma_buf_fd(struct ion_client *client, int fd)
-{
- struct dma_buf *dmabuf;
- struct ion_handle *handle;
-
- dmabuf = dma_buf_get(fd);
- if (IS_ERR(dmabuf))
- return ERR_CAST(dmabuf);
-
- handle = ion_import_dma_buf(client, dmabuf);
- dma_buf_put(dmabuf);
- return handle;
-}
-EXPORT_SYMBOL(ion_import_dma_buf_fd);
-
int ion_query_heaps(struct ion_client *client, struct ion_heap_query *query)
{
struct ion_device *dev = client->dev;
diff --git a/drivers/staging/android/uapi/ion.h
b/drivers/staging/android/uapi/ion.h
index 8ff471d..3a59044 100644
--- a/d
[RFC PATCHv2 06/21] staging: android: ion: Call dma_map_sg for syncing and mapping
Technically, calling dma_buf_map_attachment should return a buffer
properly dma_mapped. Add calls to dma_map_sg to begin_cpu_access to
ensure this happens. As a side effect, this lets Ion buffers take
advantage of the dma_buf sync ioctls.
Signed-off-by: Laura Abbott
---
drivers/staging/android/ion/ion.c | 151 ++---
drivers/staging/android/ion/ion_priv.h | 1 +
2 files changed, 103 insertions(+), 49 deletions(-)
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index 7dcb8a9..6aac935 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -162,6 +162,7 @@ static struct ion_buffer *ion_buffer_create(struct ion_heap
*heap,
buffer->dev = dev;
buffer->size = len;
INIT_LIST_HEAD(&buffer->vmas);
+ INIT_LIST_HEAD(&buffer->attachments);
mutex_init(&buffer->lock);
/*
* this will set up dma addresses for the sglist -- it is not
@@ -796,10 +797,6 @@ void ion_client_destroy(struct ion_client *client)
}
EXPORT_SYMBOL(ion_client_destroy);
-static void ion_buffer_sync_for_device(struct ion_buffer *buffer,
- struct device *dev,
- enum dma_data_direction direction);
-
static struct sg_table *dup_sg_table(struct sg_table *table)
{
struct sg_table *new_table;
@@ -826,22 +823,89 @@ static struct sg_table *dup_sg_table(struct sg_table
*table)
return new_table;
}
+static void free_duped_table(struct sg_table *table)
+{
+ sg_free_table(table);
+ kfree(table);
+}
+
+struct ion_dma_buf_attachment {
+ struct device *dev;
+ struct sg_table *table;
+ struct list_head list;
+};
+
+static int ion_dma_buf_attach(struct dma_buf *dmabuf, struct device *dev,
+ struct dma_buf_attachment *attachment)
+{
+ struct ion_dma_buf_attachment *a;
+ struct sg_table *table;
+ struct ion_buffer *buffer = dmabuf->priv;
+
+ a = kzalloc(sizeof(*a), GFP_KERNEL);
+ if (!a)
+ return -ENOMEM;
+
+ table = dup_sg_table(buffer->sg_table);
+ if (IS_ERR(table)) {
+ kfree(a);
+ return -ENOMEM;
+ }
+
+ a->table = table;
+ a->dev = dev;
+ INIT_LIST_HEAD(&a->list);
+
+ attachment->priv = a;
+
+ mutex_lock(&buffer->lock);
+ list_add(&a->list, &buffer->attachments);
+ mutex_unlock(&buffer->lock);
+
+ return 0;
+}
+
+static void ion_dma_buf_detatch(struct dma_buf *dmabuf,
+ struct dma_buf_attachment *attachment)
+{
+ struct ion_dma_buf_attachment *a = attachment->priv;
+ struct ion_buffer *buffer = dmabuf->priv;
+
+ free_duped_table(a->table);
+ mutex_lock(&buffer->lock);
+ list_del(&a->list);
+ mutex_unlock(&buffer->lock);
+
+ kfree(a);
+}
+
+
static struct sg_table *ion_map_dma_buf(struct dma_buf_attachment *attachment,
enum dma_data_direction direction)
{
- struct dma_buf *dmabuf = attachment->dmabuf;
- struct ion_buffer *buffer = dmabuf->priv;
+ struct ion_dma_buf_attachment *a = attachment->priv;
+ struct sg_table *table;
+ int ret;
+
+ table = a->table;
- ion_buffer_sync_for_device(buffer, attachment->dev, direction);
- return dup_sg_table(buffer->sg_table);
+ if (!dma_map_sg(attachment->dev, table->sgl, table->nents,
+ direction)){
+ ret = -ENOMEM;
+ goto err;
+ }
+ return table;
+
+err:
+ free_duped_table(table);
+ return ERR_PTR(ret);
}
static void ion_unmap_dma_buf(struct dma_buf_attachment *attachment,
struct sg_table *table,
enum dma_data_direction direction)
{
- sg_free_table(table);
- kfree(table);
+ dma_unmap_sg(attachment->dev, table->sgl, table->nents, direction);
}
void ion_pages_sync_for_device(struct device *dev, struct page *page,
@@ -865,38 +929,6 @@ struct ion_vma_list {
struct vm_area_struct *vma;
};
-static void ion_buffer_sync_for_device(struct ion_buffer *buffer,
- struct device *dev,
- enum dma_data_direction dir)
-{
- struct ion_vma_list *vma_list;
- int pages = PAGE_ALIGN(buffer->size) / PAGE_SIZE;
- int i;
-
- pr_debug("%s: syncing for device %s\n", __func__,
-dev ? dev_name(dev) : "null");
-
- if (!ion_buffer_fault_user_mappings(buffer))
- return;
-
- mutex_lock(&buffer->lock);
- for (i = 0; i < pages; i++) {
- struct page *page = buffer->pages[i];
-
- if (ion_buffer_page_is_dirty(page))
- ion_pages_sync_for_device(dev, ion_buffer_page(page),
-
[RFC PATCHv2 07/21] staging: android: ion: Remove page faulting support
The new method of syncing with dma_map means that the page faulting sync
implementation is no longer applicable. Remove it.
Signed-off-by: Laura Abbott
---
drivers/staging/android/ion/ion.c | 117 --
1 file changed, 117 deletions(-)
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index 6aac935..226ea1f 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -42,37 +42,11 @@
#include "ion_priv.h"
#include "compat_ion.h"
-bool ion_buffer_fault_user_mappings(struct ion_buffer *buffer)
-{
- return (buffer->flags & ION_FLAG_CACHED) &&
- !(buffer->flags & ION_FLAG_CACHED_NEEDS_SYNC);
-}
-
bool ion_buffer_cached(struct ion_buffer *buffer)
{
return !!(buffer->flags & ION_FLAG_CACHED);
}
-static inline struct page *ion_buffer_page(struct page *page)
-{
- return (struct page *)((unsigned long)page & ~(1UL));
-}
-
-static inline bool ion_buffer_page_is_dirty(struct page *page)
-{
- return !!((unsigned long)page & 1UL);
-}
-
-static inline void ion_buffer_page_dirty(struct page **page)
-{
- *page = (struct page *)((unsigned long)(*page) | 1UL);
-}
-
-static inline void ion_buffer_page_clean(struct page **page)
-{
- *page = (struct page *)((unsigned long)(*page) & ~(1UL));
-}
-
/* this function should only be called while dev->lock is held */
static void ion_buffer_add(struct ion_device *dev,
struct ion_buffer *buffer)
@@ -140,25 +114,6 @@ static struct ion_buffer *ion_buffer_create(struct
ion_heap *heap,
buffer->dev = dev;
buffer->size = len;
- if (ion_buffer_fault_user_mappings(buffer)) {
- int num_pages = PAGE_ALIGN(buffer->size) / PAGE_SIZE;
- struct scatterlist *sg;
- int i, j, k = 0;
-
- buffer->pages = vmalloc(sizeof(struct page *) * num_pages);
- if (!buffer->pages) {
- ret = -ENOMEM;
- goto err1;
- }
-
- for_each_sg(table->sgl, sg, table->nents, i) {
- struct page *page = sg_page(sg);
-
- for (j = 0; j < sg->length / PAGE_SIZE; j++)
- buffer->pages[k++] = page++;
- }
- }
-
buffer->dev = dev;
buffer->size = len;
INIT_LIST_HEAD(&buffer->vmas);
@@ -924,69 +879,6 @@ void ion_pages_sync_for_device(struct device *dev, struct
page *page,
dma_sync_sg_for_device(dev, &sg, 1, dir);
}
-struct ion_vma_list {
- struct list_head list;
- struct vm_area_struct *vma;
-};
-
-static int ion_vm_fault(struct vm_fault *vmf)
-{
- struct ion_buffer *buffer = vmf->vma->vm_private_data;
- unsigned long pfn;
- int ret;
-
- mutex_lock(&buffer->lock);
- ion_buffer_page_dirty(buffer->pages + vmf->pgoff);
- BUG_ON(!buffer->pages || !buffer->pages[vmf->pgoff]);
-
- pfn = page_to_pfn(ion_buffer_page(buffer->pages[vmf->pgoff]));
- ret = vm_insert_pfn(vmf->vma, vmf->address, pfn);
- mutex_unlock(&buffer->lock);
- if (ret)
- return VM_FAULT_ERROR;
-
- return VM_FAULT_NOPAGE;
-}
-
-static void ion_vm_open(struct vm_area_struct *vma)
-{
- struct ion_buffer *buffer = vma->vm_private_data;
- struct ion_vma_list *vma_list;
-
- vma_list = kmalloc(sizeof(*vma_list), GFP_KERNEL);
- if (!vma_list)
- return;
- vma_list->vma = vma;
- mutex_lock(&buffer->lock);
- list_add(&vma_list->list, &buffer->vmas);
- mutex_unlock(&buffer->lock);
- pr_debug("%s: adding %p\n", __func__, vma);
-}
-
-static void ion_vm_close(struct vm_area_struct *vma)
-{
- struct ion_buffer *buffer = vma->vm_private_data;
- struct ion_vma_list *vma_list, *tmp;
-
- pr_debug("%s\n", __func__);
- mutex_lock(&buffer->lock);
- list_for_each_entry_safe(vma_list, tmp, &buffer->vmas, list) {
- if (vma_list->vma != vma)
- continue;
- list_del(&vma_list->list);
- kfree(vma_list);
- pr_debug("%s: deleting %p\n", __func__, vma);
- break;
- }
- mutex_unlock(&buffer->lock);
-}
-
-static const struct vm_operations_struct ion_vma_ops = {
- .open = ion_vm_open,
- .close = ion_vm_close,
- .fault = ion_vm_fault,
-};
-
static int ion_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma)
{
struct ion_buffer *buffer = dmabuf->priv;
@@ -998,15 +890,6 @@ static int ion_mmap(struct dma_buf *dmabuf, struct
vm_area_struct *vma)
return -EINVAL;
}
- if (ion_buffer_fault_user_mappings(buffer)) {
- vma->vm_flags |= VM_IO | VM_PFNMAP | VM_DONTEXPAND |
- VM_DONTDUMP;
- vma->vm_private_data = buffer;
[RFC PATCHv2 08/21] staging: android: ion: Remove crufty cache support
Now that we call dma_map in the dma_buf API callbacks there is no need
to use the existing cache APIs. Remove the sync ioctl and the existing
bad dma_sync calls. Explicit caching can be handled with the dma_buf
sync API.
Signed-off-by: Laura Abbott
---
drivers/staging/android/ion/compat_ion.c| 1 -
drivers/staging/android/ion/ion-ioctl.c | 6
drivers/staging/android/ion/ion.c | 40 -
drivers/staging/android/ion/ion_carveout_heap.c | 6
drivers/staging/android/ion/ion_chunk_heap.c| 6
drivers/staging/android/ion/ion_page_pool.c | 3 --
drivers/staging/android/ion/ion_priv.h | 13
drivers/staging/android/ion/ion_system_heap.c | 5
drivers/staging/android/uapi/ion.h | 10 ---
9 files changed, 90 deletions(-)
diff --git a/drivers/staging/android/ion/compat_ion.c
b/drivers/staging/android/ion/compat_ion.c
index 9a978d2..b892d3a 100644
--- a/drivers/staging/android/ion/compat_ion.c
+++ b/drivers/staging/android/ion/compat_ion.c
@@ -186,7 +186,6 @@ long compat_ion_ioctl(struct file *filp, unsigned int cmd,
unsigned long arg)
case ION_IOC_SHARE:
case ION_IOC_MAP:
case ION_IOC_IMPORT:
- case ION_IOC_SYNC:
return filp->f_op->unlocked_ioctl(filp, cmd,
(unsigned long)compat_ptr(arg));
default:
diff --git a/drivers/staging/android/ion/ion-ioctl.c
b/drivers/staging/android/ion/ion-ioctl.c
index 5b2e93f..e096bcd 100644
--- a/drivers/staging/android/ion/ion-ioctl.c
+++ b/drivers/staging/android/ion/ion-ioctl.c
@@ -51,7 +51,6 @@ static int validate_ioctl_arg(unsigned int cmd, union
ion_ioctl_arg *arg)
static unsigned int ion_ioctl_dir(unsigned int cmd)
{
switch (cmd) {
- case ION_IOC_SYNC:
case ION_IOC_FREE:
case ION_IOC_CUSTOM:
return _IOC_WRITE;
@@ -146,11 +145,6 @@ long ion_ioctl(struct file *filp, unsigned int cmd,
unsigned long arg)
data.handle.handle = handle->id;
break;
}
- case ION_IOC_SYNC:
- {
- ret = ion_sync_for_device(client, data.fd.fd);
- break;
- }
case ION_IOC_CUSTOM:
{
if (!dev->custom_ioctl)
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index 226ea1f..8757164 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -863,22 +863,6 @@ static void ion_unmap_dma_buf(struct dma_buf_attachment
*attachment,
dma_unmap_sg(attachment->dev, table->sgl, table->nents, direction);
}
-void ion_pages_sync_for_device(struct device *dev, struct page *page,
- size_t size, enum dma_data_direction dir)
-{
- struct scatterlist sg;
-
- sg_init_table(&sg, 1);
- sg_set_page(&sg, page, size, 0);
- /*
-* This is not correct - sg_dma_address needs a dma_addr_t that is valid
-* for the targeted device, but this works on the currently targeted
-* hardware.
-*/
- sg_dma_address(&sg) = page_to_phys(page);
- dma_sync_sg_for_device(dev, &sg, 1, dir);
-}
-
static int ion_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma)
{
struct ion_buffer *buffer = dmabuf->priv;
@@ -1097,30 +1081,6 @@ struct ion_handle *ion_import_dma_buf_fd(struct
ion_client *client, int fd)
}
EXPORT_SYMBOL(ion_import_dma_buf_fd);
-int ion_sync_for_device(struct ion_client *client, int fd)
-{
- struct dma_buf *dmabuf;
- struct ion_buffer *buffer;
-
- dmabuf = dma_buf_get(fd);
- if (IS_ERR(dmabuf))
- return PTR_ERR(dmabuf);
-
- /* if this memory came from ion */
- if (dmabuf->ops != &dma_buf_ops) {
- pr_err("%s: can not sync dmabuf from another exporter\n",
- __func__);
- dma_buf_put(dmabuf);
- return -EINVAL;
- }
- buffer = dmabuf->priv;
-
- dma_sync_sg_for_device(NULL, buffer->sg_table->sgl,
- buffer->sg_table->nents, DMA_BIDIRECTIONAL);
- dma_buf_put(dmabuf);
- return 0;
-}
-
int ion_query_heaps(struct ion_client *client, struct ion_heap_query *query)
{
struct ion_device *dev = client->dev;
diff --git a/drivers/staging/android/ion/ion_carveout_heap.c
b/drivers/staging/android/ion/ion_carveout_heap.c
index 9bf8e98..e0e360f 100644
--- a/drivers/staging/android/ion/ion_carveout_heap.c
+++ b/drivers/staging/android/ion/ion_carveout_heap.c
@@ -100,10 +100,6 @@ static void ion_carveout_heap_free(struct ion_buffer
*buffer)
ion_heap_buffer_zero(buffer);
- if (ion_buffer_cached(buffer))
- dma_sync_sg_for_device(NULL, table->sgl, table->nents,
- DMA_BIDIRECTIONAL);
-
ion_carveout_free(heap, paddr, buffer->size);
[RFC PATCHv2 03/21] staging: android: ion: Remove dmap_cnt
The reference counting of dma_map calls was removed. Remove the
associated counter field as well.
Signed-off-by: Laura Abbott
---
drivers/staging/android/ion/ion_priv.h | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/staging/android/ion/ion_priv.h
b/drivers/staging/android/ion/ion_priv.h
index 5b3059c..46d3ff5 100644
--- a/drivers/staging/android/ion/ion_priv.h
+++ b/drivers/staging/android/ion/ion_priv.h
@@ -44,7 +44,6 @@
* @lock: protects the buffers cnt fields
* @kmap_cnt: number of times the buffer is mapped to the kernel
* @vaddr: the kernel mapping if kmap_cnt is not zero
- * @dmap_cnt: number of times the buffer is mapped for dma
* @sg_table: the sg table for the buffer if dmap_cnt is not zero
* @pages: flat array of pages in the buffer -- used by fault
* handler and only valid for buffers that are faulted in
@@ -70,7 +69,6 @@ struct ion_buffer {
struct mutex lock;
int kmap_cnt;
void *vaddr;
- int dmap_cnt;
struct sg_table *sg_table;
struct page **pages;
struct list_head vmas;
--
2.7.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC PATCHv2 02/21] cma: Introduce cma_for_each_area
Frameworks (e.g. Ion) may want to iterate over each possible CMA area to
allow for enumeration. Introduce a function to allow a callback.
Signed-off-by: Laura Abbott
---
include/linux/cma.h | 2 ++
mm/cma.c| 14 ++
2 files changed, 16 insertions(+)
diff --git a/include/linux/cma.h b/include/linux/cma.h
index d41d1f8..3e8fbf5 100644
--- a/include/linux/cma.h
+++ b/include/linux/cma.h
@@ -34,4 +34,6 @@ extern int cma_init_reserved_mem(phys_addr_t base,
phys_addr_t size,
extern struct page *cma_alloc(struct cma *cma, size_t count, unsigned int
align,
gfp_t gfp_mask);
extern bool cma_release(struct cma *cma, const struct page *pages, unsigned
int count);
+
+extern int cma_for_each_area(int (*it)(struct cma *cma, void *data), void
*data);
#endif
diff --git a/mm/cma.c b/mm/cma.c
index 0d187b1..9a040e1 100644
--- a/mm/cma.c
+++ b/mm/cma.c
@@ -498,3 +498,17 @@ bool cma_release(struct cma *cma, const struct page
*pages, unsigned int count)
return true;
}
+
+int cma_for_each_area(int (*it)(struct cma *cma, void *data), void *data)
+{
+ int i;
+
+ for (i = 0; i < cma_area_count; i++) {
+ int ret = it(&cma_areas[i], data);
+
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
--
2.7.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC PATCHv2 09/21] staging: android: ion: Remove custom ioctl interface
Ion is now moving towards a unified interfact. This makes the custom
ioctl interface unneeded. Remove it.
Signed-off-by: Laura Abbott
---
drivers/staging/android/ion/compat_ion.c | 40
drivers/staging/android/ion/ion-ioctl.c | 11 -
drivers/staging/android/ion/ion.c| 6 +
drivers/staging/android/ion/ion_priv.h | 8 +--
drivers/staging/android/uapi/ion.h | 21 -
5 files changed, 2 insertions(+), 84 deletions(-)
diff --git a/drivers/staging/android/ion/compat_ion.c
b/drivers/staging/android/ion/compat_ion.c
index b892d3a..5b192ea 100644
--- a/drivers/staging/android/ion/compat_ion.c
+++ b/drivers/staging/android/ion/compat_ion.c
@@ -30,11 +30,6 @@ struct compat_ion_allocation_data {
compat_int_t handle;
};
-struct compat_ion_custom_data {
- compat_uint_t cmd;
- compat_ulong_t arg;
-};
-
struct compat_ion_handle_data {
compat_int_t handle;
};
@@ -43,8 +38,6 @@ struct compat_ion_handle_data {
struct compat_ion_allocation_data)
#define COMPAT_ION_IOC_FREE_IOWR(ION_IOC_MAGIC, 1, \
struct compat_ion_handle_data)
-#define COMPAT_ION_IOC_CUSTOM _IOWR(ION_IOC_MAGIC, 6, \
- struct compat_ion_custom_data)
static int compat_get_ion_allocation_data(
struct compat_ion_allocation_data __user *data32,
@@ -105,22 +98,6 @@ static int compat_put_ion_allocation_data(
return err;
}
-static int compat_get_ion_custom_data(
- struct compat_ion_custom_data __user *data32,
- struct ion_custom_data __user *data)
-{
- compat_uint_t cmd;
- compat_ulong_t arg;
- int err;
-
- err = get_user(cmd, &data32->cmd);
- err |= put_user(cmd, &data->cmd);
- err |= get_user(arg, &data32->arg);
- err |= put_user(arg, &data->arg);
-
- return err;
-};
-
long compat_ion_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
{
long ret;
@@ -166,23 +143,6 @@ long compat_ion_ioctl(struct file *filp, unsigned int cmd,
unsigned long arg)
return filp->f_op->unlocked_ioctl(filp, ION_IOC_FREE,
(unsigned long)data);
}
- case COMPAT_ION_IOC_CUSTOM: {
- struct compat_ion_custom_data __user *data32;
- struct ion_custom_data __user *data;
- int err;
-
- data32 = compat_ptr(arg);
- data = compat_alloc_user_space(sizeof(*data));
- if (!data)
- return -EFAULT;
-
- err = compat_get_ion_custom_data(data32, data);
- if (err)
- return err;
-
- return filp->f_op->unlocked_ioctl(filp, ION_IOC_CUSTOM,
- (unsigned long)data);
- }
case ION_IOC_SHARE:
case ION_IOC_MAP:
case ION_IOC_IMPORT:
diff --git a/drivers/staging/android/ion/ion-ioctl.c
b/drivers/staging/android/ion/ion-ioctl.c
index e096bcd..2b475bf 100644
--- a/drivers/staging/android/ion/ion-ioctl.c
+++ b/drivers/staging/android/ion/ion-ioctl.c
@@ -26,7 +26,6 @@ union ion_ioctl_arg {
struct ion_fd_data fd;
struct ion_allocation_data allocation;
struct ion_handle_data handle;
- struct ion_custom_data custom;
struct ion_heap_query query;
};
@@ -52,7 +51,6 @@ static unsigned int ion_ioctl_dir(unsigned int cmd)
{
switch (cmd) {
case ION_IOC_FREE:
- case ION_IOC_CUSTOM:
return _IOC_WRITE;
default:
return _IOC_DIR(cmd);
@@ -62,7 +60,6 @@ static unsigned int ion_ioctl_dir(unsigned int cmd)
long ion_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
{
struct ion_client *client = filp->private_data;
- struct ion_device *dev = client->dev;
struct ion_handle *cleanup_handle = NULL;
int ret = 0;
unsigned int dir;
@@ -145,14 +142,6 @@ long ion_ioctl(struct file *filp, unsigned int cmd,
unsigned long arg)
data.handle.handle = handle->id;
break;
}
- case ION_IOC_CUSTOM:
- {
- if (!dev->custom_ioctl)
- return -ENOTTY;
- ret = dev->custom_ioctl(client, data.custom.cmd,
- data.custom.arg);
- break;
- }
case ION_IOC_HEAP_QUERY:
ret = ion_query_heaps(client, &data.query);
break;
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index 8757164..125c537 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -1347,10 +1347,7 @@ void ion_device_add_heap(struct ion_device *dev, struct
ion_heap *h
[RFC PATCHv2 05/21] staging: android: ion: Duplicate sg_table
Ion currently returns a single sg_table on each dma_map call. This is
incorrect for later usage.
Signed-off-by: Laura Abbott
---
drivers/staging/android/ion/ion.c | 30 +-
1 file changed, 29 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index c2adfe1..7dcb8a9 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -800,6 +800,32 @@ static void ion_buffer_sync_for_device(struct ion_buffer
*buffer,
struct device *dev,
enum dma_data_direction direction);
+static struct sg_table *dup_sg_table(struct sg_table *table)
+{
+ struct sg_table *new_table;
+ int ret, i;
+ struct scatterlist *sg, *new_sg;
+
+ new_table = kzalloc(sizeof(*new_table), GFP_KERNEL);
+ if (!new_table)
+ return ERR_PTR(-ENOMEM);
+
+ ret = sg_alloc_table(new_table, table->nents, GFP_KERNEL);
+ if (ret) {
+ kfree(new_table);
+ return ERR_PTR(-ENOMEM);
+ }
+
+ new_sg = new_table->sgl;
+ for_each_sg(table->sgl, sg, table->nents, i) {
+ memcpy(new_sg, sg, sizeof(*sg));
+ sg->dma_address = 0;
+ new_sg = sg_next(new_sg);
+ }
+
+ return new_table;
+}
+
static struct sg_table *ion_map_dma_buf(struct dma_buf_attachment *attachment,
enum dma_data_direction direction)
{
@@ -807,13 +833,15 @@ static struct sg_table *ion_map_dma_buf(struct
dma_buf_attachment *attachment,
struct ion_buffer *buffer = dmabuf->priv;
ion_buffer_sync_for_device(buffer, attachment->dev, direction);
- return buffer->sg_table;
+ return dup_sg_table(buffer->sg_table);
}
static void ion_unmap_dma_buf(struct dma_buf_attachment *attachment,
struct sg_table *table,
enum dma_data_direction direction)
{
+ sg_free_table(table);
+ kfree(table);
}
void ion_pages_sync_for_device(struct device *dev, struct page *page,
--
2.7.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC PATCHv2 00/21] Ion clean in preparation for moving out of staging
Hi, This is v2 of the series to do some serious Ion clean up in preparation for moving out of staging. I got good feedback last time so this series mostly attempts to address that feedback and do more still more cleanup. Highlights: - All calls to DMA APIs should now be with a real actual proper device structure - Patch to stop setting sg_dma_address manually now included - Fix for a bug in the query interface - Removal of custom ioctl interface - Removal of import interface - Removal of any notion of using Ion as an in kernel interface. - Cleanup of ABI so compat interface is no longer needed - Deletion of a bit more platform code - Combined heap enumeration and heap registration code up so there are fewer layers of abstraction - Some general cleanup and header reduction. - Removal of both the ion_client and ion_handle structures since these mostly become redundant. As a result, Ion only returns a dma_buf fd. The overall result is that the only Ion interfaces are the query ioctl and the alloc ioctl. The following are still TODOs/open problems: - Sumit's comments about the CMA naming. - Bindings/platform for chunk and carveout heap - There was some discussion about making the sg_table duplication generic. I got bogged down in handling some of the edge cases for generic handling so I put this aside. Making it generic is still something that should happen. - More fine-grained support for restricting heap access. There are good arguments to be made for having a way for having good integration with selinux and other policy mechanisms. - While not on the original list, there is still no good good test standalone test framework. I noticed that the existing ion_test was fairly generic so I proposed moving it to dma_buf. Daniel Vetter suggested just using the VGEM module instead. Ideally, the tests can live as part of some other existing test set (drm tests maybe?) Feedback appreciated as always. Thanks, Laura Laura Abbott (21): cma: Store a name in the cma structure cma: Introduce cma_for_each_area staging: android: ion: Remove dmap_cnt staging: android: ion: Remove alignment from allocation field staging: android: ion: Duplicate sg_table staging: android: ion: Call dma_map_sg for syncing and mapping staging: android: ion: Remove page faulting support staging: android: ion: Remove crufty cache support staging: android: ion: Remove custom ioctl interface staging: android: ion: Remove import interface staging: android: ion: Remove duplicate ION_IOC_MAP staging: android: ion: Remove old platform support staging: android: ion: Use CMA APIs directly staging: android: ion: Stop butchering the DMA address staging: android: ion: Break the ABI in the name of forward progress staging: android: ion: Get rid of ion_phys_addr_t staging: android: ion: Collapse internal header files staging: android: ion: Rework heap registration/enumeration staging: android: ion: Drop ion_map_kernel interface staging: android: ion: Remove ion_handle and ion_client staging: android: ion: Set query return value drivers/base/dma-contiguous.c |5 +- drivers/staging/android/ion/Kconfig| 56 +- drivers/staging/android/ion/Makefile | 18 +- drivers/staging/android/ion/compat_ion.c | 195 drivers/staging/android/ion/compat_ion.h | 29 - drivers/staging/android/ion/hisilicon/Kconfig |5 - drivers/staging/android/ion/hisilicon/Makefile |1 - drivers/staging/android/ion/hisilicon/hi6220_ion.c | 113 -- drivers/staging/android/ion/ion-ioctl.c| 85 +- drivers/staging/android/ion/ion.c | 1164 +++- drivers/staging/android/ion/ion.h | 393 +-- drivers/staging/android/ion/ion_carveout_heap.c| 37 +- drivers/staging/android/ion/ion_chunk_heap.c | 27 +- drivers/staging/android/ion/ion_cma_heap.c | 125 +-- drivers/staging/android/ion/ion_dummy_driver.c | 156 --- drivers/staging/android/ion/ion_heap.c | 68 -- drivers/staging/android/ion/ion_of.c | 184 drivers/staging/android/ion/ion_of.h | 37 - drivers/staging/android/ion/ion_page_pool.c|6 +- drivers/staging/android/ion/ion_priv.h | 473 drivers/staging/android/ion/ion_system_heap.c | 53 +- drivers/staging/android/ion/ion_test.c | 305 - drivers/staging/android/ion/tegra/Makefile |1 - drivers/staging/android/ion/tegra/tegra_ion.c | 80 -- drivers/staging/android/uapi/ion.h | 86 +- drivers/staging/android/uapi/ion_test.h| 69 -- include/linux/cma.h|6 +- mm/cma.c | 25 +- mm/cma.h |1 + mm/cma_debug.c |2 +- 30
Re: [PATCH 2/2] drm/pl111: Initial drm/kms driver for pl111
Russell King - ARM Linux writes: > On Fri, Mar 17, 2017 at 03:47:42PM -0700, Eric Anholt wrote: >> This is a modesetting driver for the pl111 CLCD display controller >> found on various ARM platforms such as the Versatile Express. The >> driver has only been tested on the bcm911360_entphn platform so far, >> with PRIME-based buffer sharing between vc4 and clcd. >> >> It reuses the existing devicetree binding, while not using quite as >> many of its properties as the fbdev driver does (those are left for >> future work). > > As we're multiplatform on ARM, and this is using the PL11x AMBA IDs, > we must ensure that it's compatible with everything that the fbdev > driver is compatible with... however, I suspect that's not worth the > effort (unless Linus W wants it?) > > If we make it PL111 specific, then we don't need to handle Integrator > CP, or the Versatile PB/AB weirdness. The only thing left is the > power etc enable bits on Realview which uses the PL111. See the > code for Realview in drivers/video/fbdev/amba-clcd-versatile.c. Restricting to PL111 for now sounds good to me. Those Realview bits look like they're turning on a power domain -- shouldn't we represent those as a regulator or a power domain? If we did, then plugging that into a panel driver sounds straightforward. (that's assuming that they're powering panel. not the controller -- I can't quite tell from the code I've browsed so far) signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: vc4: remove redundant check of plane being non-null
Colin King writes:
> From: Colin Ian King
>
> The pointer plane is always null on the error path at label 'fail'
> hence the check if it is non-null is redundant. We can therefore
> remove the check and the destruction of plane as well as the fail
> error path and instead just return an -ENOMEM ERR_PTR.
>
> Detected by CoverityScan, CID#1339532 ("Logically Dead Code")
>
> Signed-off-by: Colin Ian King
Reviewed and applied. Thanks!
signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/vc4: use platform_register_drivers
Philipp Zabel writes: > Use platform_register_drivers instead of open coding the iteration over > component platform drivers in the vc4_drv module. Reviewed and applied. Thanks! signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/2] drm/vc4: Fix misleading name of the continuous flag.
The logic was all right in the end, the name was just backwards.
Signed-off-by: Eric Anholt
---
drivers/gpu/drm/vc4/vc4_dsi.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_dsi.c b/drivers/gpu/drm/vc4/vc4_dsi.c
index 3bad8bad32a2..deba62008fd0 100644
--- a/drivers/gpu/drm/vc4/vc4_dsi.c
+++ b/drivers/gpu/drm/vc4/vc4_dsi.c
@@ -820,18 +820,18 @@ static void vc4_dsi_latch_ulps(struct vc4_dsi *dsi, bool
latch)
/* Enters or exits Ultra Low Power State. */
static void vc4_dsi_ulps(struct vc4_dsi *dsi, bool ulps)
{
- bool continuous = dsi->mode_flags & MIPI_DSI_CLOCK_NON_CONTINUOUS;
- u32 phyc_ulps = ((continuous ? DSI_PORT_BIT(PHYC_CLANE_ULPS) : 0) |
+ bool non_continuous = dsi->mode_flags & MIPI_DSI_CLOCK_NON_CONTINUOUS;
+ u32 phyc_ulps = ((non_continuous ? DSI_PORT_BIT(PHYC_CLANE_ULPS) : 0) |
DSI_PHYC_DLANE0_ULPS |
(dsi->lanes > 1 ? DSI_PHYC_DLANE1_ULPS : 0) |
(dsi->lanes > 2 ? DSI_PHYC_DLANE2_ULPS : 0) |
(dsi->lanes > 3 ? DSI_PHYC_DLANE3_ULPS : 0));
- u32 stat_ulps = ((continuous ? DSI1_STAT_PHY_CLOCK_ULPS : 0) |
+ u32 stat_ulps = ((non_continuous ? DSI1_STAT_PHY_CLOCK_ULPS : 0) |
DSI1_STAT_PHY_D0_ULPS |
(dsi->lanes > 1 ? DSI1_STAT_PHY_D1_ULPS : 0) |
(dsi->lanes > 2 ? DSI1_STAT_PHY_D2_ULPS : 0) |
(dsi->lanes > 3 ? DSI1_STAT_PHY_D3_ULPS : 0));
- u32 stat_stop = ((continuous ? DSI1_STAT_PHY_CLOCK_STOP : 0) |
+ u32 stat_stop = ((non_continuous ? DSI1_STAT_PHY_CLOCK_STOP : 0) |
DSI1_STAT_PHY_D0_STOP |
(dsi->lanes > 1 ? DSI1_STAT_PHY_D1_STOP : 0) |
(dsi->lanes > 2 ? DSI1_STAT_PHY_D2_STOP : 0) |
--
2.11.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] drm/vc4: Fix DSI T_INIT timing.
The DPHY spec requires a much larger T_INIT than I was specifying before. In the absence of clear specs from the slave of what their timing is, just use the value that the firmware was using. Signed-off-by: Eric Anholt --- drivers/gpu/drm/vc4/vc4_dsi.c | 12 +++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/vc4/vc4_dsi.c b/drivers/gpu/drm/vc4/vc4_dsi.c index 160f981d1cf4..3bad8bad32a2 100644 --- a/drivers/gpu/drm/vc4/vc4_dsi.c +++ b/drivers/gpu/drm/vc4/vc4_dsi.c @@ -1094,7 +1094,17 @@ static void vc4_dsi_encoder_enable(struct drm_encoder *encoder) DSI_HS_DLT4_TRAIL) | VC4_SET_FIELD(0, DSI_HS_DLT4_ANLAT)); - DSI_PORT_WRITE(HS_DLT5, VC4_SET_FIELD(dsi_hs_timing(ui_ns, 1000, 5000), + /* T_INIT is how long STOP is driven after power-up to +* indicate to the slave (also coming out of power-up) that +* master init is complete, and should be greater than the +* maximum of two value: T_INIT,MASTER and T_INIT,SLAVE. The +* D-PHY spec gives a minimum 100us for T_INIT,MASTER and +* T_INIT,SLAVE, while allowing protocols on top of it to give +* greater minimums. The vc4 firmware uses an extremely +* conservative 5ms, and we maintain that here. +*/ + DSI_PORT_WRITE(HS_DLT5, VC4_SET_FIELD(dsi_hs_timing(ui_ns, + 5 * 1000 * 1000, 0), DSI_HS_DLT5_INIT)); DSI_PORT_WRITE(HS_DLT6, -- 2.11.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/2] drm/pl111: Initial drm/kms driver for pl111
so need to write
+ * a panel driver (which may be as simple as an entry in
+ * panel-simple.c).
+ *
+ * The driver currently doesn't expose the cursor. The DRM API for
+ * cursors requires support for 64x64 ARGB cursor images, while
+ * the hardware can only support 64x64 monochrome with masking
+ * cursors. While one could imagine trying to hack something together
+ * to look at the ARGB and program reasonable in monochrome, we
+ * just don't expose the cursor at all instead, and leave cursor
+ * support to the X11 software cursor layer.
+ *
+ * TODO:
+ *
+ * - Fix race between setting plane base address and getting IRQ for
+ * vsync firing the pageflip completion.
+ *
+ * - Expose the correct set of formats we can support based on the
+ * "arm,pl11x,tft-r0g0b0-pads" DT property.
+ *
+ * - Use the "max-memory-bandwidth" DT property to filter the
+ * supported formats.
+ *
+ * - Read back hardware state at boot to skip reprogramming the
+ * hardware when doing a no-op modeset.
+ *
+ * - Use the internal clock divisor to reduce power consumption by
+ * using HCLK (apb_pclk) when appropriate.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+#include
+#include
+
+#include "pl111_drm.h"
+
+#define DRIVER_DESC "DRM module for PL111"
+
+struct drm_mode_config_funcs mode_config_funcs = {
+ .fb_create = drm_fb_cma_create,
+ .atomic_check = drm_atomic_helper_check,
+ .atomic_commit = drm_atomic_helper_commit,
+};
+
+static int pl111_modeset_init(struct drm_device *dev)
+{
+ struct drm_mode_config *mode_config;
+ struct pl111_drm_dev_private *priv = dev->dev_private;
+ int ret = 0;
+
+ if (!priv)
+ return -EINVAL;
+
+ drm_mode_config_init(dev);
+ mode_config = &dev->mode_config;
+ mode_config->funcs = &mode_config_funcs;
+ mode_config->min_width = 1;
+ mode_config->max_width = 1024;
+ mode_config->min_height = 1;
+ mode_config->max_height = 768;
+
+ ret = pl111_primary_plane_init(dev);
+ if (ret != 0) {
+ dev_err(dev->dev, "Failed to init primary plane\n");
+ goto out_config;
+ }
+
+ ret = pl111_crtc_create(dev);
+ if (ret) {
+ dev_err(dev->dev, "Failed to create crtc\n");
+ goto out_config;
+ }
+
+ ret = pl111_connector_create(dev);
+ if (ret) {
+ dev_err(dev->dev, "Failed to create pl111_drm_connector\n");
+ goto out_config;
+ }
+
+ ret = pl111_encoder_init(dev);
+ if (ret) {
+ dev_err(dev->dev, "Failed to create pl111_drm_encoder\n");
+ goto out_config;
+ }
+
+ ret = drm_mode_connector_attach_encoder(&priv->connector.connector,
+ &priv->encoder);
+ if (ret != 0) {
+ dev_err(dev->dev, "Failed to attach encoder\n");
+ goto out_config;
+ }
+
+ priv->connector.connector.encoder = &priv->encoder;
+
+ ret = drm_vblank_init(dev, 1);
+ if (ret != 0) {
+ dev_err(dev->dev, "Failed to init vblank\n");
+ goto out_config;
+ }
+
+ drm_mode_config_reset(dev);
+
+ priv->fbdev = drm_fbdev_cma_init(dev, 32,
+dev->mode_config.num_connector);
+
+ drm_kms_helper_poll_init(dev);
+
+ goto finish;
+
+out_config:
+ drm_mode_config_cleanup(dev);
+finish:
+ return ret;
+}
+
+static const struct file_operations drm_fops = {
+ .owner = THIS_MODULE,
+ .open = drm_open,
+ .release = drm_release,
+ .unlocked_ioctl = drm_ioctl,
+ .mmap = drm_gem_cma_mmap,
+ .poll = drm_poll,
+ .read = drm_read,
+};
+
+static void pl111_lastclose(struct drm_device *dev)
+{
+ struct pl111_drm_dev_private *priv = dev->dev_private;
+
+ drm_fbdev_cma_restore_mode(priv->fbdev);
+}
+
+static struct drm_driver pl111_drm_driver = {
+ .driver_features =
+ DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME | DRIVER_ATOMIC,
+ .lastclose = pl111_lastclose,
+ .ioctls = NULL,
+ .fops = &drm_fops,
+ .name = "pl111_drm",
+ .desc = DRIVER_DESC,
+ .date = "20170317",
+ .major = 1,
+ .minor = 0,
+ .patchlevel = 0,
+ .dumb_create = pl111_dumb_create,
+ .dumb_destroy = drm_gem_dumb_destroy,
+ .dumb_map_offset = drm_gem_cma_dumb_map_offset,
+ .gem_free_object = drm_gem_cma_free_object,
+ .gem_vm_ops = &drm_gem_cma_vm_ops,
+
+ .prime_handle_to_fd = drm_gem_prime_handle_to_fd,
+ .prime_fd_to_handle = drm_gem_prime_fd_to_handle
[PATCH 0/2] ARM CLCD DRM driver
This is a resurrection of Tom Cooksey's old CLCD DRM driver, which I needed in order to do dmabuf buffer sharing from VC4 to the CLCD display on the bcm911360_entphn platform. With a couple of hacks to the X Server, I've got it (sometimes) running glxgears on vc4 and displaying on CLCD. The panel power sequencing part is not thoroughly tested. My DRM panel driver (not included) for this board is still a stub and doesn't do the power on SPI reset sequence. There are a few TODO entries left, which are noted in the documentation. My primary concern with this driver is that, since it binds to the same device as fbdev but isn't a complete replacement, people might turn it on and break their display setup. I figure until someone wants to turn DRM CLCD on in a multi-platform defconfig, we ought to be fine with that, though. My plan would be for this to go through the drm-misc tree once it's ready. For anyone interested in looking through the history of what I changed from Tom's v1 patch, https://github.com/anholt/linux/tree/bcm11360-clcd has it (and all the rest of the platform bits I've needed). Eric Anholt (1): video: ARM CLCD: Move registers to a separate header. Tom Cooksey (1): drm/pl111: Initial drm/kms driver for pl111 Documentation/gpu/index.rst | 1 + Documentation/gpu/pl111.rst | 6 + drivers/gpu/drm/Kconfig | 2 + drivers/gpu/drm/Makefile| 1 + drivers/gpu/drm/pl111/Kconfig | 12 ++ drivers/gpu/drm/pl111/Makefile | 8 + drivers/gpu/drm/pl111/pl111_connector.c | 127 ++ drivers/gpu/drm/pl111/pl111_crtc.c | 239 ++ drivers/gpu/drm/pl111/pl111_drm.h | 64 +++ drivers/gpu/drm/pl111/pl111_drv.c | 292 drivers/gpu/drm/pl111/pl111_encoder.c | 50 ++ drivers/gpu/drm/pl111/pl111_gem.c | 35 drivers/gpu/drm/pl111/pl111_plane.c | 167 ++ include/linux/amba/clcd-regs.h | 76 + include/linux/amba/clcd.h | 68 +--- 15 files changed, 1081 insertions(+), 67 deletions(-) create mode 100644 Documentation/gpu/pl111.rst create mode 100644 drivers/gpu/drm/pl111/Kconfig create mode 100644 drivers/gpu/drm/pl111/Makefile create mode 100644 drivers/gpu/drm/pl111/pl111_connector.c create mode 100644 drivers/gpu/drm/pl111/pl111_crtc.c create mode 100644 drivers/gpu/drm/pl111/pl111_drm.h create mode 100644 drivers/gpu/drm/pl111/pl111_drv.c create mode 100644 drivers/gpu/drm/pl111/pl111_encoder.c create mode 100644 drivers/gpu/drm/pl111/pl111_gem.c create mode 100644 drivers/gpu/drm/pl111/pl111_plane.c create mode 100644 include/linux/amba/clcd-regs.h -- 2.11.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] video: ARM CLCD: Move registers to a separate header.
We'd like to reuse these register definitions for the DRM CLCD driver, but there's a bunch of fbdev-specific code in the current header. Signed-off-by: Eric Anholt --- include/linux/amba/clcd-regs.h | 76 ++ include/linux/amba/clcd.h | 68 + 2 files changed, 77 insertions(+), 67 deletions(-) create mode 100644 include/linux/amba/clcd-regs.h diff --git a/include/linux/amba/clcd-regs.h b/include/linux/amba/clcd-regs.h new file mode 100644 index ..1b924c4a63ab --- /dev/null +++ b/include/linux/amba/clcd-regs.h @@ -0,0 +1,76 @@ +/* + * David A Rusling + * + * Copyright (C) 2001 ARM Limited + * + * This file is subject to the terms and conditions of the GNU General Public + * License. See the file COPYING in the main directory of this archive + * for more details. + */ + +/* + * CLCD Controller Internal Register addresses + */ +#define CLCD_TIM0 0x +#define CLCD_TIM1 0x0004 +#define CLCD_TIM2 0x0008 +#define CLCD_TIM3 0x000c +#define CLCD_UBAS 0x0010 +#define CLCD_LBAS 0x0014 + +#define CLCD_PL110_IENB0x0018 +#define CLCD_PL110_CNTL0x001c +#define CLCD_PL110_STAT0x0020 +#define CLCD_PL110_INTR0x0024 +#define CLCD_PL110_UCUR0x0028 +#define CLCD_PL110_LCUR0x002C + +#define CLCD_PL111_CNTL0x0018 +#define CLCD_PL111_IENB0x001c +#define CLCD_PL111_RIS 0x0020 +#define CLCD_PL111_MIS 0x0024 +#define CLCD_PL111_ICR 0x0028 +#define CLCD_PL111_UCUR0x002c +#define CLCD_PL111_LCUR0x0030 + +#define CLCD_PALL 0x0200 +#define CLCD_PALETTE 0x0200 + +#define TIM2_CLKSEL(1 << 5) +#define TIM2_IVS (1 << 11) +#define TIM2_IHS (1 << 12) +#define TIM2_IPC (1 << 13) +#define TIM2_IOE (1 << 14) +#define TIM2_BCD (1 << 26) + +#define CNTL_LCDEN (1 << 0) +#define CNTL_LCDBPP1 (0 << 1) +#define CNTL_LCDBPP2 (1 << 1) +#define CNTL_LCDBPP4 (2 << 1) +#define CNTL_LCDBPP8 (3 << 1) +#define CNTL_LCDBPP16 (4 << 1) +#define CNTL_LCDBPP16_565 (6 << 1) +#define CNTL_LCDBPP16_444 (7 << 1) +#define CNTL_LCDBPP24 (5 << 1) +#define CNTL_LCDBW (1 << 4) +#define CNTL_LCDTFT(1 << 5) +#define CNTL_LCDMONO8 (1 << 6) +#define CNTL_LCDDUAL (1 << 7) +#define CNTL_BGR (1 << 8) +#define CNTL_BEBO (1 << 9) +#define CNTL_BEPO (1 << 10) +#define CNTL_LCDPWR(1 << 11) +#define CNTL_LCDVCOMP(x) ((x) << 12) +#define CNTL_LDMAFIFOTIME (1 << 15) +#define CNTL_WATERMARK (1 << 16) + +/* ST Microelectronics variant bits */ +#define CNTL_ST_1XBPP_444 0x0 +#define CNTL_ST_1XBPP_5551 (1 << 17) +#define CNTL_ST_1XBPP_565 (1 << 18) +#define CNTL_ST_CDWID_12 0x0 +#define CNTL_ST_CDWID_16 (1 << 19) +#define CNTL_ST_CDWID_18 (1 << 20) +#define CNTL_ST_CDWID_24 ((1 << 19)|(1 << 20)) +#define CNTL_ST_CEAEN (1 << 21) +#define CNTL_ST_LCDBPP24_PACKED(6 << 1) diff --git a/include/linux/amba/clcd.h b/include/linux/amba/clcd.h index 1035879b322c..d0c3be77c18e 100644 --- a/include/linux/amba/clcd.h +++ b/include/linux/amba/clcd.h @@ -10,73 +10,7 @@ * for more details. */ #include - -/* - * CLCD Controller Internal Register addresses - */ -#define CLCD_TIM0 0x -#define CLCD_TIM1 0x0004 -#define CLCD_TIM2 0x0008 -#define CLCD_TIM3 0x000c -#define CLCD_UBAS 0x0010 -#define CLCD_LBAS 0x0014 - -#define CLCD_PL110_IENB0x0018 -#define CLCD_PL110_CNTL0x001c -#define CLCD_PL110_STAT0x0020 -#define CLCD_PL110_INTR0x0024 -#define CLCD_PL110_UCUR0x0028 -#define CLCD_PL110_LCUR0x002C - -#define CLCD_PL111_CNTL0x0018 -#define CLCD_PL111_IENB0x001c -#define CLCD_PL111_RIS 0x0020 -#define CLCD_PL111_MIS 0x0024 -#define CLCD_PL111_ICR 0x0028 -#define CLCD_PL111_UCUR0x002c -#define CLCD_PL111_LCUR0x0030 - -#define CLCD_PALL 0x0200 -#define CLCD_PALETTE 0x0200 - -#define TIM2_CLKSEL(1 << 5) -#define TIM2_IVS (1 << 11) -#define TIM2_IHS (1 << 12) -#define TIM2_IPC (1 << 13) -#define TIM2_IOE (1 << 14) -#define TIM2_BCD (1 << 26) - -#define CNTL_LCDEN (1 << 0) -#define CNT
Re: [RFC libdrm 0/2] Replace the build system with meson
On Fri, Mar 17, 2017 at 5:15 AM, Dylan Baker wrote: > Quoting Marek Olšák (2017-03-16 18:53:59) >> On Fri, Mar 17, 2017 at 12:11 AM, Dylan Baker wrote: >> > Quoting Marek Olšák (2017-03-16 15:36:26) >> >> Is there a way not to use ninja with meson, because ninja redirects >> >> all stderr output from gcc to stdout, which breaks many development >> >> environments that expect errors in stderr? >> >> >> >> I'm basically saying that if ninja can't keep gcc errors in stderr, I >> >> wouldn't like any project that I might be involved in to require ninja >> >> for building. >> >> >> >> Marek >> > >> > There is no way to use another backend on Linux, and meson will not support >> > Make. Ninja is a big part of the appeal here, since it is faster than make >> > is. >> > Are there particular tools you know don't work with ninja? It seems like >> > in the >> > 7+ years since ninja came out that someone would have fixed the tools, or >> > that >> > some stream redirection could be used to fix the problem, "ninja 1>&2"? >> >> I actually read some thread about it and the conclusion seemed to be >> that ninja developers don't care. I have no other option than to >> believe that ninja was made for automated build bots, not for >> development. >> >> Some editors expect that errors and only errors go to stderr and all >> other garbage info goes to stdout. This is something I can't change. >> >> Marek > > And I found this: > https://groups.google.com/forum/#!topic/ninja-build/4syh2jzXWcI > > Which leads me to believe that they would be responsive to a patch, the core > team just doesn't have a use for it. There is in fact a patch already written > (linked in that thread), that just needs someone to clean it up and propose it > for merge. Well, I guess I can use that. It's still not nice to force some people to use out-of-tree builds of ninja. Marek ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 1/4] drm: Mark up accesses of vblank->enabled outside of its spinlock
On Fri, Mar 17, 2017 at 08:20:27PM +, Chris Wilson wrote:
> Order the update to vblank->enabled after the timestamp is primed so
> that a concurrent unlocked reader will only see the vblank->enabled with
> the current timestamp.
>
> v2: vblank->enable is guarded by dev->vbl_lock not
> dev->vblank_time_lock, update the READ_ONCE accordingly.
The locking is indeed very confusing, and I don't know if it even makes
sense anymore. But this looks at least as sane as anything else here :)
For the series:
Reviewed-by: Ville Syrjälä
>
> Do not add a READ_ONCE(vblank->enabled) inside the interrupt handler to
> avoid missing an interrupt whilst racing with enable_vblank()
>
> Signed-off-by: Chris Wilson
> Cc: Ville Syrjälä
> Cc: Daniel Vetter
> ---
> drivers/gpu/drm/drm_irq.c | 23 ++-
> 1 file changed, 14 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> index 53a526c7b24d..c47e07c89136 100644
> --- a/drivers/gpu/drm/drm_irq.c
> +++ b/drivers/gpu/drm/drm_irq.c
> @@ -325,6 +325,8 @@ static void vblank_disable_and_save(struct drm_device
> *dev, unsigned int pipe)
> struct drm_vblank_crtc *vblank = &dev->vblank[pipe];
> unsigned long irqflags;
>
> + assert_spin_locked(&dev->vbl_lock);
> +
> /* Prevent vblank irq processing while disabling vblank irqs,
>* so no updates of timestamps or count can happen after we've
>* disabled. Needed to prevent races in case of delayed irq's.
> @@ -336,10 +338,8 @@ static void vblank_disable_and_save(struct drm_device
> *dev, unsigned int pipe)
>* calling the ->disable_vblank() operation in atomic context with the
>* hardware potentially runtime suspended.
>*/
> - if (vblank->enabled) {
> + if (cmpxchg_relaxed(&vblank->enabled, true, false))
> __disable_vblank(dev, pipe);
> - vblank->enabled = false;
> - }
>
> /*
>* Always update the count and timestamp to maintain the
> @@ -384,7 +384,7 @@ void drm_vblank_cleanup(struct drm_device *dev)
> for (pipe = 0; pipe < dev->num_crtcs; pipe++) {
> struct drm_vblank_crtc *vblank = &dev->vblank[pipe];
>
> - WARN_ON(vblank->enabled &&
> + WARN_ON(READ_ONCE(vblank->enabled) &&
> drm_core_check_feature(dev, DRIVER_MODESET));
>
> del_timer_sync(&vblank->disable_timer);
> @@ -1105,11 +1105,16 @@ static int drm_vblank_enable(struct drm_device *dev,
> unsigned int pipe)
>*/
> ret = __enable_vblank(dev, pipe);
> DRM_DEBUG("enabling vblank on crtc %u, ret: %d\n", pipe, ret);
> - if (ret)
> + if (ret) {
> atomic_dec(&vblank->refcount);
> - else {
> - vblank->enabled = true;
> + } else {
> drm_update_vblank_count(dev, pipe, 0);
> + /* drm_update_vblank_count() includes a wmb so we just
> + * need to ensure that the compiler emits the write
> + * to mark the vblank as enabled after the call
> + * to drm_update_vblank_count().
> + */
> + WRITE_ONCE(vblank->enabled, true);
> }
> }
>
> @@ -1517,7 +1522,7 @@ static int drm_queue_vblank_event(struct drm_device
> *dev, unsigned int pipe,
>* vblank disable, so no need for further locking. The reference from
>* drm_vblank_get() protects against vblank disable from another source.
>*/
> - if (!vblank->enabled) {
> + if (!READ_ONCE(vblank->enabled)) {
> ret = -EINVAL;
> goto err_unlock;
> }
> @@ -1644,7 +1649,7 @@ int drm_wait_vblank(struct drm_device *dev, void *data,
> DRM_WAIT_ON(ret, vblank->queue, 3 * HZ,
> (((drm_vblank_count(dev, pipe) -
> vblwait->request.sequence) <= (1 << 23)) ||
> - !vblank->enabled ||
> + !READ_ONCE(vblank->enabled) ||
>!dev->irq_enabled));
> }
>
> --
> 2.11.0
--
Ville Syrjälä
Intel OTC
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 99841] Switching to VT freezes X only on a dual screen
https://bugs.freedesktop.org/show_bug.cgi?id=99841
--- Comment #19 from Peter Mattern ---
The patch is fixing the problem on an ATI system as well here (Radeon HD 7480D
/ APU A4-5300, Arch Linux, kernel 4.10.{1-3}).
Is there any way to track when it will be included in mainline?
--
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 v2 2/4] drm: vblank cannot be enabled if dev->irq_enabled is false
Since we cannot enable the vblank if !dev->irq_enabled, we assert that
checking for both !vblank->enabled and !dev->irq_enabled is tautological
and only need the former. The only time it may differ is when racing
with drm_irq_uninstall(), but that will then disable the vblank and
wakeup the waiters.
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/drm_irq.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index c47e07c89136..a164cf51d093 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -1647,10 +1647,9 @@ int drm_wait_vblank(struct drm_device *dev, void *data,
DRM_DEBUG("waiting on vblank count %u, crtc %u\n",
vblwait->request.sequence, pipe);
DRM_WAIT_ON(ret, vblank->queue, 3 * HZ,
- (((drm_vblank_count(dev, pipe) -
- vblwait->request.sequence) <= (1 << 23)) ||
-!READ_ONCE(vblank->enabled) ||
-!dev->irq_enabled));
+ (drm_vblank_count(dev, pipe) -
+vblwait->request.sequence) <= (1 << 23) ||
+ !READ_ONCE(vblank->enabled));
}
if (ret != -EINTR) {
--
2.11.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 3/4] drm: Refactor vblank sequence number comparison
Move the repeated (a - b) <= (1 << 23) to its own function.
Signed-off-by: Chris Wilson
Cc: Ville Syrjälä
Cc: Daniel Vetter
---
drivers/gpu/drm/drm_irq.c | 14 +-
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index a164cf51d093..253505da57bd 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -1492,6 +1492,11 @@ int drm_legacy_modeset_ctl(struct drm_device *dev, void
*data,
return 0;
}
+static inline bool vblank_passed(u32 seq, u32 ref)
+{
+ return (seq - ref) <= (1 << 23);
+}
+
static int drm_queue_vblank_event(struct drm_device *dev, unsigned int pipe,
union drm_wait_vblank *vblwait,
struct drm_file *file_priv)
@@ -1542,7 +1547,7 @@ static int drm_queue_vblank_event(struct drm_device *dev,
unsigned int pipe,
vblwait->request.sequence);
e->event.sequence = vblwait->request.sequence;
- if ((seq - vblwait->request.sequence) <= (1 << 23)) {
+ if (vblank_passed(seq, vblwait->request.sequence)) {
drm_vblank_put(dev, pipe);
send_vblank_event(dev, e, seq, &now);
vblwait->reply.sequence = seq;
@@ -1632,9 +1637,8 @@ int drm_wait_vblank(struct drm_device *dev, void *data,
}
if ((flags & _DRM_VBLANK_NEXTONMISS) &&
- (seq - vblwait->request.sequence) <= (1 << 23)) {
+ vblank_passed(seq, vblwait->request.sequence))
vblwait->request.sequence = seq + 1;
- }
if (flags & _DRM_VBLANK_EVENT) {
/* must hold on to the vblank ref until the event fires
@@ -1647,8 +1651,8 @@ int drm_wait_vblank(struct drm_device *dev, void *data,
DRM_DEBUG("waiting on vblank count %u, crtc %u\n",
vblwait->request.sequence, pipe);
DRM_WAIT_ON(ret, vblank->queue, 3 * HZ,
- (drm_vblank_count(dev, pipe) -
-vblwait->request.sequence) <= (1 << 23) ||
+ vblank_passed(drm_vblank_count(dev, pipe),
+ vblwait->request.sequence) ||
!READ_ONCE(vblank->enabled));
}
--
2.11.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 4/4] drm: Peek at the current counter/timestamp for vblank queries
Bypass all the spinlocks and return the last timestamp and counter from
the last vblank if the driver delcares that it is accurate (and stable
across on/off), and the vblank is currently enabled.
This is dependent upon the both the hardware and driver to provide the
proper barriers to facilitate reading our bookkeeping outside of the
vblank interrupt and outside of the explicit vblank locks.
Signed-off-by: Chris Wilson
Cc: Ville Syrjälä
Cc: Daniel Vetter
Cc: Michel Dänzer
Cc: Laurent Pinchart
Cc: Dave Airlie ,
Cc: Mario Kleiner
---
drivers/gpu/drm/drm_irq.c | 26 ++
1 file changed, 26 insertions(+)
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index 253505da57bd..846401548ec9 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -1569,6 +1569,17 @@ static int drm_queue_vblank_event(struct drm_device
*dev, unsigned int pipe,
return ret;
}
+static bool drm_wait_vblank_is_query(union drm_wait_vblank *vblwait)
+{
+ if (vblwait->request.sequence)
+ return false;
+
+ return _DRM_VBLANK_RELATIVE ==
+ (vblwait->request.type & (_DRM_VBLANK_TYPES_MASK |
+ _DRM_VBLANK_EVENT |
+ _DRM_VBLANK_NEXTONMISS));
+}
+
/*
* Wait for VBLANK.
*
@@ -1618,6 +1629,21 @@ int drm_wait_vblank(struct drm_device *dev, void *data,
vblank = &dev->vblank[pipe];
+ /* If the counter is currently enabled and accurate, short-circuit
+* queries to return the cached timestamp of the last vblank.
+*/
+ if (dev->vblank_disable_immediate &&
+ drm_wait_vblank_is_query(vblwait) &&
+ READ_ONCE(vblank->enabled)) {
+ struct timeval now;
+
+ vblwait->reply.sequence =
+ drm_vblank_count_and_time(dev, pipe, &now);
+ vblwait->reply.tval_sec = now.tv_sec;
+ vblwait->reply.tval_usec = now.tv_usec;
+ return 0;
+ }
+
ret = drm_vblank_get(dev, pipe);
if (ret) {
DRM_DEBUG("crtc %d failed to acquire vblank counter, %d\n",
pipe, ret);
--
2.11.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 1/4] drm: Mark up accesses of vblank->enabled outside of its spinlock
Order the update to vblank->enabled after the timestamp is primed so
that a concurrent unlocked reader will only see the vblank->enabled with
the current timestamp.
v2: vblank->enable is guarded by dev->vbl_lock not
dev->vblank_time_lock, update the READ_ONCE accordingly.
Do not add a READ_ONCE(vblank->enabled) inside the interrupt handler to
avoid missing an interrupt whilst racing with enable_vblank()
Signed-off-by: Chris Wilson
Cc: Ville Syrjälä
Cc: Daniel Vetter
---
drivers/gpu/drm/drm_irq.c | 23 ++-
1 file changed, 14 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index 53a526c7b24d..c47e07c89136 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -325,6 +325,8 @@ static void vblank_disable_and_save(struct drm_device *dev,
unsigned int pipe)
struct drm_vblank_crtc *vblank = &dev->vblank[pipe];
unsigned long irqflags;
+ assert_spin_locked(&dev->vbl_lock);
+
/* Prevent vblank irq processing while disabling vblank irqs,
* so no updates of timestamps or count can happen after we've
* disabled. Needed to prevent races in case of delayed irq's.
@@ -336,10 +338,8 @@ static void vblank_disable_and_save(struct drm_device
*dev, unsigned int pipe)
* calling the ->disable_vblank() operation in atomic context with the
* hardware potentially runtime suspended.
*/
- if (vblank->enabled) {
+ if (cmpxchg_relaxed(&vblank->enabled, true, false))
__disable_vblank(dev, pipe);
- vblank->enabled = false;
- }
/*
* Always update the count and timestamp to maintain the
@@ -384,7 +384,7 @@ void drm_vblank_cleanup(struct drm_device *dev)
for (pipe = 0; pipe < dev->num_crtcs; pipe++) {
struct drm_vblank_crtc *vblank = &dev->vblank[pipe];
- WARN_ON(vblank->enabled &&
+ WARN_ON(READ_ONCE(vblank->enabled) &&
drm_core_check_feature(dev, DRIVER_MODESET));
del_timer_sync(&vblank->disable_timer);
@@ -1105,11 +1105,16 @@ static int drm_vblank_enable(struct drm_device *dev,
unsigned int pipe)
*/
ret = __enable_vblank(dev, pipe);
DRM_DEBUG("enabling vblank on crtc %u, ret: %d\n", pipe, ret);
- if (ret)
+ if (ret) {
atomic_dec(&vblank->refcount);
- else {
- vblank->enabled = true;
+ } else {
drm_update_vblank_count(dev, pipe, 0);
+ /* drm_update_vblank_count() includes a wmb so we just
+* need to ensure that the compiler emits the write
+* to mark the vblank as enabled after the call
+* to drm_update_vblank_count().
+*/
+ WRITE_ONCE(vblank->enabled, true);
}
}
@@ -1517,7 +1522,7 @@ static int drm_queue_vblank_event(struct drm_device *dev,
unsigned int pipe,
* vblank disable, so no need for further locking. The reference from
* drm_vblank_get() protects against vblank disable from another source.
*/
- if (!vblank->enabled) {
+ if (!READ_ONCE(vblank->enabled)) {
ret = -EINVAL;
goto err_unlock;
}
@@ -1644,7 +1649,7 @@ int drm_wait_vblank(struct drm_device *dev, void *data,
DRM_WAIT_ON(ret, vblank->queue, 3 * HZ,
(((drm_vblank_count(dev, pipe) -
vblwait->request.sequence) <= (1 << 23)) ||
-!vblank->enabled ||
+!READ_ONCE(vblank->enabled) ||
!dev->irq_enabled));
}
--
2.11.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: add check for plane functions
On 2017-03-17 03:55 AM, Shirish S wrote:
update_plane() and disable_plane() functions
assoiciated with setting plane are called
without any check, causing kernel panic.
This patch adds the required check to avoid it.
Change-Id: I0d6792608b33e674c217388aa57c4b7d680d9bc7
Signed-off-by: Shirish S
---
drivers/gpu/drm/drm_plane.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index 249c0ae..f675f8b 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -456,6 +456,12 @@ static int __setplane_internal(struct drm_plane *plane,
{
int ret = 0;
+ if (plane->funcs->disable_plane == NULL ||
+ plane->funcs->update_plane == NULL) {
Thanks, Shirish, for sending these out on amd-gfx and dri-devel.
I've had a second look at this. It seems like there are a bunch of other
places that assume these functions are implemented by drivers. Does this
check serve any purpose other than as an immediate step when
implementing universal planes?
Harry
+ DRM_ERROR("plane funcs not implemented\n");
+ ret = -EPERM;
+ goto out;
+ }
/* No fb means shut it down */
if (!fb) {
plane->old_fb = plane->fb;
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] drm: Peek at the current counter/timestamp for vblank queries
On Fri, Mar 17, 2017 at 11:49:32AM +0200, Ville Syrjälä wrote:
> On Thu, Mar 16, 2017 at 11:47:49PM +, Chris Wilson wrote:
> > Bypass all the spinlocks and return the last timestamp and counter from
> > the last vblank if the driver delcares that it is accurate (and stable
> > across on/off), and the vblank is currently enabled.
> >
> > This is dependent upon the both the hardware and driver to provide the
> > proper barriers to facilitate reading our bookkeeping outside of the
> > vblank interrupt and outside of the explicit vblank locks.
> >
> > Signed-off-by: Chris Wilson
> > Cc: Ville Syrjälä
> > Cc: Daniel Vetter
> > Cc: Michel Dänzer
> > Cc: Laurent Pinchart
> > Cc: Dave Airlie ,
> > Cc: Mario Kleiner
> > ---
> > drivers/gpu/drm/drm_irq.c | 26 ++
> > 1 file changed, 26 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> > index 4cc9352ab6a8..b4bd361a2bcc 100644
> > --- a/drivers/gpu/drm/drm_irq.c
> > +++ b/drivers/gpu/drm/drm_irq.c
> > @@ -1562,6 +1562,17 @@ static int drm_queue_vblank_event(struct drm_device
> > *dev, unsigned int pipe,
> > return ret;
> > }
> >
> > +static bool drm_wait_vblank_is_query(union drm_wait_vblank *vblwait)
> > +{
> > + if (vblwait->request.sequence)
> > + return false;
> > +
> > + return _DRM_VBLANK_RELATIVE ==
> > + (vblwait->request.type & (_DRM_VBLANK_TYPES_MASK |
> > + _DRM_VBLANK_EVENT |
> > + _DRM_VBLANK_NEXTONMISS));
> > +}
> > +
> > /*
> > * Wait for VBLANK.
> > *
> > @@ -1611,6 +1622,21 @@ int drm_wait_vblank(struct drm_device *dev, void
> > *data,
> >
> > vblank = &dev->vblank[pipe];
> >
> > + /* If the counter is currently enabled and accurate, short-circuit
> > queries
> > +* to return the cached timestamp of the last vblank.
> > +*/
> > + if (dev->vblank_disable_immediate &&
> > + drm_wait_vblank_is_query(vblwait) &&
> > + vblank->enabled) {
> > + struct timeval now;
> > +
>
> Do we want a comment here as well stating that the seqlock
> already has the rmb?
I didn't find it enlightening. Added READ_ONCE(vblank->enabled).
-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
Re: [PATCH] drm: add check for plane functions
Ville Syrjälä writes:
> On Fri, Mar 17, 2017 at 05:57:52PM +0100, Daniel Vetter wrote:
>> On Fri, Mar 17, 2017 at 01:08:43PM +0200, Ville Syrjälä wrote:
>> > On Fri, Mar 17, 2017 at 03:46:34PM +0530, Shirish S wrote:
>> > > On Fri, Mar 17, 2017 at 3:26 PM, Ville Syrjälä
>> > > wrote:
>> > > > On Fri, Mar 17, 2017 at 01:25:08PM +0530, Shirish S wrote:
>> > > >> update_plane() and disable_plane() functions
>> > > >> assoiciated with setting plane are called
>> > > >> without any check, causing kernel panic.
>> > > >
>> > > > Why are you registering a plane without the funcs?
>> > > >
>> > > Basically, enabling planes and making them fully functional is
>> > > generally a 2 -step process,
>> > > so i suggest for new drivers wanting to implement/re-design planes,
>> > > would like to tap
>> > > the flow at enabling(listing caps) and later at ensuring it works.
>> >
>> > I don't think there's much point in exposing something that
>> > doesn't work. And even if you do, you could always just use
>> > stub functions.
>>
>> Yes, just wire up stub functions if you want to enable planes with
>> multi-step patch series.
>>
>> > > I noticed that there is a underlying assumption only for
>> > > plane->(funcs) are implemented, whereas for
>> > > other function for crtc/connector/encoder function calls there is a
>> > > sanity check(or WARN_ON) through out the framework.
>> > >
>> > > I believe this check wont cause any performance/functional impact.
>> > > Please let me know if am missing anything.
>> > > And further more help developers to focus on enabling planes via
>> > > various tests without causing reboots/system hangs.
>> >
>> > I don't particularly like adding more unconditional runtime checks
>> > that just to protect developers from themselves. If you really
>> > think there's value in these, then at least add the checks into
>> > the plane init codepath so that it's a one time cost.
>> >
>> > The same approach could be used for all the other non-optional
>> > hooks. Otherwise the same WARN_ON()s would have to be sprinkled
>> > all over the place, and there's always the risk of missing a few
>> > codepaths that call a specific hook.
>>
>> I think for these here there's negative value - it allows developers to
>> create completely broken planes. Stub functions really seem like a much
>> better idea.
>
> I was thinking
>
> drm_whatever_init()
> {
> if (WARN_ON(!funcs->mandatory_thing))
> return -EINVAL;
> }
>
> rather than putting the WARN_ON()s around each call of
> funcs->mandatory_thing().
>
> That will fail gracefully (which I guess is what people are after here),
> and gives the developer a clear message what's missing.
Having this in our init functions for funcs and helpers would have saved
me tons of time in vc4 and clcd.
signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH libdrm] atomic: fix atomic_add_unless() fallback's return value
[+ Lionel]
On 16 March 2017 at 16:46, Eric Engestrom wrote:
> On Thursday, 2017-03-16 00:08:59 +, Eric Engestrom wrote:
>> According to the kernel documentation:
>> Returns non-zero if @v was not @u, and zero otherwise.
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100077
>> Fixes: 63fc571863aa64683400 ("atomic: add atomic_add_unless()")
>> Signed-off-by: David Shao
>> Reviewed-by: Eric Engestrom
>> Signed-off-by: Eric Engestrom
>
> That last s-o-b was added automatically, by mistake. I'll remove it when
> pushing this.
>
> This patch seems trivial to verify, can someone confirm this is right?
>
> /me is about ~94% sure, but doesn't know much about atomic ops and their
> return value convention.
>
> I checked the kernel and it does what libdrm does after this patch, so
> either one should be fixed; I'm assuming kernel is right and libdrm
> needs fixing, hence this patch.
>
Changing the function matches the documentation, yet it doesn't [seem
to] align with how it's used.
I'm fairly sure that this will break libdrm_intel.
Lionel, can you take a look at this patch please ?
https://patchwork.freedesktop.org/patch/144252/
Emil
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/amdgpu: remove redundant outer loop and remove commented out code
On Fri, Mar 17, 2017 at 10:37 AM, Colin King wrote:
> From: Colin Ian King
>
> The outer loop is redundant and can be removed as it is doing nothing
> useful. Also remove some commented out code that is not being used.
>
> Detected by CoverityScan, CID#1402073
>
> Signed-off-by: Colin Ian King
Applied. thanks!
Alex
> ---
> drivers/gpu/drm/amd/amdgpu/vce_v2_0.c | 27 +++
> 1 file changed, 7 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v2_0.c
> b/drivers/gpu/drm/amd/amdgpu/vce_v2_0.c
> index 9ea99348e493..72dea663f113 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vce_v2_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vce_v2_0.c
> @@ -273,24 +273,14 @@ static int vce_v2_0_start(struct amdgpu_device *adev)
>
> static int vce_v2_0_stop(struct amdgpu_device *adev)
> {
> - int i, j;
> + int i;
> int status;
>
> if (vce_v2_0_lmi_clean(adev)) {
> DRM_INFO("vce is not idle \n");
> return 0;
> }
> -/*
> - for (i = 0; i < 10; ++i) {
> - for (j = 0; j < 100; ++j) {
> - status = RREG32(mmVCE_FW_REG_STATUS);
> - if (!(status & 1))
> - break;
> - mdelay(1);
> - }
> - break;
> - }
> -*/
> +
> if (vce_v2_0_wait_for_idle(adev)) {
> DRM_INFO("VCE is busy, Can't set clock gateing");
> return 0;
> @@ -299,14 +289,11 @@ static int vce_v2_0_stop(struct amdgpu_device *adev)
> /* Stall UMC and register bus before resetting VCPU */
> WREG32_P(mmVCE_LMI_CTRL2, 1 << 8, ~(1 << 8));
>
> - for (i = 0; i < 10; ++i) {
> - for (j = 0; j < 100; ++j) {
> - status = RREG32(mmVCE_LMI_STATUS);
> - if (status & 0x240)
> - break;
> - mdelay(1);
> - }
> - break;
> + for (i = 0; i < 100; ++i) {
> + status = RREG32(mmVCE_LMI_STATUS);
> + if (status & 0x240)
> + break;
> + mdelay(1);
> }
>
> WREG32_P(mmVCE_VCPU_CNTL, 0, ~0x80001);
> --
> 2.11.0
>
> ___
> 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
[PATCH] drm/msm: for array in-fences, check if all backing fences are from our own context before waiting
Use the dma_fence_match_context helper to check if all backing fences
are from our own context, in which case we don't have to wait.
Signed-off-by: Philipp Zabel
Cc: Rob Clark
Cc: Gustavo Padovan
---
Not sure if this can be handled exactly the same as for etnaviv. This depends on
d5b72a2123df ("dma-fence: add dma_fence_match_context helper").
---
drivers/gpu/drm/msm/msm_gem_submit.c | 10 --
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c
b/drivers/gpu/drm/msm/msm_gem_submit.c
index 1172fe7a9252c..507a6f5b911f0 100644
--- a/drivers/gpu/drm/msm/msm_gem_submit.c
+++ b/drivers/gpu/drm/msm/msm_gem_submit.c
@@ -439,17 +439,15 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void
*data,
goto out;
}
- /* TODO if we get an array-fence due to userspace merging
multiple
-* fences, we need a way to determine if all the backing fences
-* are from our own context..
+ /*
+* Wait if the fence is from a foreign context, or if the fence
+* array contains any fence from a foreign context.
*/
-
- if (in_fence->context != gpu->fctx->context) {
+ if (!dma_fence_match_context(in_fence, gpu->fctx->context)) {
ret = dma_fence_wait(in_fence, true);
if (ret)
goto out;
}
-
}
if (!(args->fence & MSM_SUBMIT_NO_IMPLICIT)) {
--
2.11.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 1/4] dma-fence: add dma_fence_match_context helper
Am 17.03.2017 um 17:34 schrieb Philipp Zabel:
Add a helper to check if all fences in a fence array are from a given
context. For convenience, the function can also handle being given a
non-array fence.
Signed-off-by: Philipp Zabel
Reviewed-by: Christian König
---
drivers/dma-buf/dma-fence-array.c | 26 ++
include/linux/dma-fence-array.h | 2 ++
2 files changed, 28 insertions(+)
diff --git a/drivers/dma-buf/dma-fence-array.c
b/drivers/dma-buf/dma-fence-array.c
index 67eb7c8fb88c3..0350829ba62e7 100644
--- a/drivers/dma-buf/dma-fence-array.c
+++ b/drivers/dma-buf/dma-fence-array.c
@@ -144,3 +144,29 @@ struct dma_fence_array *dma_fence_array_create(int
num_fences,
return array;
}
EXPORT_SYMBOL(dma_fence_array_create);
+
+/**
+ * dma_fence_match_context - Check if all fences are from the given context
+ * @fence: [in]fence or fence array
+ * @context: [in]fence context to check all fences against
+ *
+ * Checks the provided fence or, for a fence array, all fences in the array
+ * against the given context. Returns false if any fence is from a different
+ * context.
+ */
+bool dma_fence_match_context(struct dma_fence *fence, u64 context)
+{
+ struct dma_fence_array *array = to_dma_fence_array(fence);
+ unsigned i;
+
+ if (!dma_fence_is_array(fence))
+ return fence->context == context;
+
+ for (i = 0; i < array->num_fences; i++) {
+ if (array->fences[i]->context != context)
+ return false;
+ }
+
+ return true;
+}
+EXPORT_SYMBOL(dma_fence_match_context);
diff --git a/include/linux/dma-fence-array.h b/include/linux/dma-fence-array.h
index 5900945f962dd..332a5420243c4 100644
--- a/include/linux/dma-fence-array.h
+++ b/include/linux/dma-fence-array.h
@@ -83,4 +83,6 @@ struct dma_fence_array *dma_fence_array_create(int num_fences,
u64 context, unsigned seqno,
bool signal_on_any);
+bool dma_fence_match_context(struct dma_fence *fence, u64 context);
+
#endif /* __LINUX_DMA_FENCE_ARRAY_H */
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm: bochs: Don't remove uninitialized fbdev framebuffer
In the same spirit of the fix for QXL in commit 861078381ba5 ("drm: qxl:
Don't alloc fbdev if emulation is not supported"), prevent the Oops in
the unbind path of Bochs if fbdev emulation is disabled.
[ 112.176009] Oops: 0002 [#1] SMP
[ 112.176009] Modules linked in: bochs_drm
[ 112.176009] CPU: 0 PID: 3002 Comm: bash Not tainted 4.11.0-rc1+ #111
[ 112.176009] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
1.9.3-20161025_171302-gandalf 04/01/2014
[ 112.176009] task: 8800743bbac0 task.stack: c9b5c000
[ 112.176009] RIP: 0010:mutex_lock+0x18/0x30
[ 112.176009] RSP: 0018:c9b5fc78 EFLAGS: 00010246
[ 112.176009] RAX: RBX: 0260 RCX:
[ 112.176009] RDX: 8800743bbac0 RSI: 8800787176e0 RDI: 0260
[ 112.176009] RBP: c9b5fc80 R08: R09:
[ 112.176009] R10: 88007b463650 R11: R12: 0260
[ 112.176009] R13: 8800787176e0 R14: a0003068 R15: 0060
[ 112.176009] FS: 7f20564c7b40() GS:88007ce0()
knlGS:
[ 112.176009] CS: 0010 DS: ES: CR0: 80050033
[ 112.176009] CR2: 0260 CR3: 6b89c000 CR4: 06f0
[ 112.176009] Call Trace:
[ 112.176009] drm_mode_object_unregister+0x1e/0x50
[ 112.176009] drm_framebuffer_unregister_private+0x15/0x20
[ 112.176009] bochs_fbdev_fini+0x57/0x70 [bochs_drm]
[ 112.176009] bochs_unload+0x16/0x50 [bochs_drm]
[ 112.176009] drm_dev_unregister+0x37/0xd0
[ 112.176009] drm_put_dev+0x31/0x60
[ 112.176009] bochs_pci_remove+0x10/0x20 [bochs_drm]
[ 112.176009] pci_device_remove+0x34/0xb0
[ 112.176009] device_release_driver_internal+0x150/0x200
[ 112.176009] device_release_driver+0xd/0x10
[ 112.176009] unbind_store+0x108/0x150
[ 112.176009] drv_attr_store+0x20/0x30
[ 112.176009] sysfs_kf_write+0x32/0x40
[ 112.176009] kernfs_fop_write+0x10b/0x190
[ 112.176009] __vfs_write+0x23/0x120
[ 112.176009] ? security_file_permission+0x36/0xb0
[ 112.176009] ? rw_verify_area+0x49/0xb0
[ 112.176009] vfs_write+0xb0/0x190
[ 112.176009] SyS_write+0x41/0xa0
[ 112.176009] entry_SYSCALL_64_fastpath+0x1a/0xa9
[ 112.176009] RIP: 0033:0x7f2055bd5620
[ 112.176009] RSP: 002b:7ffed2f487d8 EFLAGS: 0246 ORIG_RAX:
0001
[ 112.176009] RAX: ffda RBX: RCX: 7f2055bd5620
[ 112.176009] RDX: 000d RSI: 00ee0008 RDI: 0001
[ 112.176009] RBP: 0001 R08: 7f2055e94760 R09: 7f20564c7b40
[ 112.176009] R10: 0073 R11: 0246 R12:
[ 112.176009] R13: 7ffed2f48d70 R14: R15:
[ 112.176009] Code: 00 00 00 55 be 02 00 00 00 48 89 e5 e8 62 fb ff ff 5d c3
55 48 89 e5 53 48 89 fb e8 53 e9 ff ff 65 48 8b 14 25 40 c4 00 00 31 c0 48
0f b1 13 48 85 c0 74 08 48 89 df e8c6 ff ff ff 5b 5d c3
[ 112.176009] RIP: mutex_lock+0x18/0x30 RSP: c9b5fc78
[ 112.176009] CR2: 0260
[ 112.205622] ---[ end trace 76189cd7a9bdd155 ]---
Signed-off-by: Gabriel Krisman Bertazi
---
drivers/gpu/drm/bochs/bochs_fbdev.c | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/bochs/bochs_fbdev.c
b/drivers/gpu/drm/bochs/bochs_fbdev.c
index a11debaad626..471bd588550b 100644
--- a/drivers/gpu/drm/bochs/bochs_fbdev.c
+++ b/drivers/gpu/drm/bochs/bochs_fbdev.c
@@ -132,6 +132,7 @@ static int bochsfb_create(struct drm_fb_helper *helper,
info->fix.smem_start = 0;
info->fix.smem_len = size;
+ bochs->fb.initialized = true;
return 0;
}
@@ -148,7 +149,6 @@ static int bochs_fbdev_destroy(struct bochs_device *bochs)
gfb->obj = NULL;
}
- drm_fb_helper_fini(&bochs->fb.helper);
drm_framebuffer_unregister_private(&gfb->base);
drm_framebuffer_cleanup(&gfb->base);
@@ -180,7 +180,6 @@ int bochs_fbdev_init(struct bochs_device *bochs)
if (ret)
goto fini;
- bochs->fb.initialized = true;
return 0;
fini:
@@ -190,9 +189,9 @@ int bochs_fbdev_init(struct bochs_device *bochs)
void bochs_fbdev_fini(struct bochs_device *bochs)
{
- if (!bochs->fb.initialized)
- return;
+ if (bochs->fb.initialized)
+ bochs_fbdev_destroy(bochs);
- bochs_fbdev_destroy(bochs);
+ drm_fb_helper_fini(&bochs->fb.helper);
bochs->fb.initialized = false;
}
--
2.11.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 2/4] drm/etnaviv: submit support for in-fences
Hi Philipp,
Thanks for your patch!
On 17 March 2017 at 22:04, Philipp Zabel wrote:
> Loosely based on commit f0a42bb5423a ("drm/msm: submit support for
> in-fences"). Unfortunately, struct drm_etnaviv_gem_submit doesn't have
> a flags field yet, so we have to extend the structure and trust that
> drm_ioctl will clear the flags for us if an older userspace only submits
> part of the struct.
>
> Signed-off-by: Philipp Zabel
Please feel free to add my
Reviewed-by: Sumit Semwal
> ---
> v2: Use dma_fence_match_context to skip waiting if all in-fences are ours.
> ---
Best,
Sumit.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 1/4] dma-fence: add dma_fence_match_context helper
Hi Philipp,
thanks for the patch!
On 17 March 2017 at 22:04, Philipp Zabel wrote:
> Add a helper to check if all fences in a fence array are from a given
> context. For convenience, the function can also handle being given a
> non-array fence.
>
> Signed-off-by: Philipp Zabel
> ---
Looks good to me, please feel free to add my
Acked-by: Sumit Semwal
> drivers/dma-buf/dma-fence-array.c | 26 ++
> include/linux/dma-fence-array.h | 2 ++
> 2 files changed, 28 insertions(+)
>
> diff --git a/drivers/dma-buf/dma-fence-array.c
> b/drivers/dma-buf/dma-fence-array.c
> index 67eb7c8fb88c3..0350829ba62e7 100644
> --- a/drivers/dma-buf/dma-fence-array.c
> +++ b/drivers/dma-buf/dma-fence-array.c
> @@ -144,3 +144,29 @@ struct dma_fence_array *dma_fence_array_create(int
> num_fences,
> return array;
> }
> EXPORT_SYMBOL(dma_fence_array_create);
> +
> +/**
> + * dma_fence_match_context - Check if all fences are from the given context
> + * @fence: [in]fence or fence array
> + * @context: [in]fence context to check all fences against
> + *
> + * Checks the provided fence or, for a fence array, all fences in the array
> + * against the given context. Returns false if any fence is from a different
> + * context.
> + */
> +bool dma_fence_match_context(struct dma_fence *fence, u64 context)
> +{
> + struct dma_fence_array *array = to_dma_fence_array(fence);
> + unsigned i;
> +
> + if (!dma_fence_is_array(fence))
> + return fence->context == context;
> +
> + for (i = 0; i < array->num_fences; i++) {
> + if (array->fences[i]->context != context)
> + return false;
> + }
> +
> + return true;
> +}
> +EXPORT_SYMBOL(dma_fence_match_context);
> diff --git a/include/linux/dma-fence-array.h b/include/linux/dma-fence-array.h
> index 5900945f962dd..332a5420243c4 100644
> --- a/include/linux/dma-fence-array.h
> +++ b/include/linux/dma-fence-array.h
> @@ -83,4 +83,6 @@ struct dma_fence_array *dma_fence_array_create(int
> num_fences,
>u64 context, unsigned seqno,
>bool signal_on_any);
>
> +bool dma_fence_match_context(struct dma_fence *fence, u64 context);
> +
> #endif /* __LINUX_DMA_FENCE_ARRAY_H */
> --
> 2.11.0
>
Best,
Sumit.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH 08/12] cma: Store a name in the cma structure
On 03/10/2017 12:53 AM, Sumit Semwal wrote:
> Hi Laura,
>
> Thanks for the patch.
>
> On 3 March 2017 at 03:14, Laura Abbott wrote:
>>
>> Frameworks that may want to enumerate CMA heaps (e.g. Ion) will find it
>> useful to have an explicit name attached to each region. Store the name
>> in each CMA structure.
>>
>> Signed-off-by: Laura Abbott
>> ---
>> drivers/base/dma-contiguous.c | 5 +++--
>> include/linux/cma.h | 4 +++-
>> mm/cma.c | 11 +--
>> mm/cma.h | 1 +
>> mm/cma_debug.c| 2 +-
>> 5 files changed, 17 insertions(+), 6 deletions(-)
>>
>
>> +const char *cma_get_name(const struct cma *cma)
>> +{
>> + return cma->name ? cma->name : "(undefined)";
>> +}
>> +
> Would it make sense to perhaps have the idx stored as the name,
> instead of 'undefined'? That would make sure that the various cma
> names are still unique.
>
Good suggestion. I'll see about cleaning that up.
>> static unsigned long cma_bitmap_aligned_mask(const struct cma *cma,
>> int align_order)
>> {
>> @@ -168,6 +173,7 @@ core_initcall(cma_init_reserved_areas);
>> */
>> int __init cma_init_reserved_mem(phys_addr_t base, phys_addr_t size,
>> unsigned int order_per_bit,
>> +const char *name,
>> struct cma **res_cma)
>> {
>
> Best regards,
> Sumit.
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 2/4] drm/etnaviv: submit support for in-fences
Hi Philipp,
2017-03-17 Philipp Zabel :
> Loosely based on commit f0a42bb5423a ("drm/msm: submit support for
> in-fences"). Unfortunately, struct drm_etnaviv_gem_submit doesn't have
> a flags field yet, so we have to extend the structure and trust that
> drm_ioctl will clear the flags for us if an older userspace only submits
> part of the struct.
>
> Signed-off-by: Philipp Zabel
> ---
> v2: Use dma_fence_match_context to skip waiting if all in-fences are ours.
> ---
> drivers/gpu/drm/etnaviv/Kconfig | 1 +
> drivers/gpu/drm/etnaviv/etnaviv_gem.h| 1 +
> drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 34
> +++-
> drivers/gpu/drm/etnaviv/etnaviv_gpu.c| 5 +++-
> drivers/gpu/drm/etnaviv/etnaviv_gpu.h| 2 +-
> include/uapi/drm/etnaviv_drm.h | 6 +
> 6 files changed, 46 insertions(+), 3 deletions(-)
Reviewed-by: Gustavo Padovan
Gustavo
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 1/4] dma-fence: add dma_fence_match_context helper
Hi Philipp, 2017-03-17 Philipp Zabel : > Add a helper to check if all fences in a fence array are from a given > context. For convenience, the function can also handle being given a > non-array fence. > > Signed-off-by: Philipp Zabel > --- > drivers/dma-buf/dma-fence-array.c | 26 ++ > include/linux/dma-fence-array.h | 2 ++ > 2 files changed, 28 insertions(+) Looks good to me. Reviewed-by: Gustavo Padovan Gustavo ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: add check for plane functions
On Fri, Mar 17, 2017 at 05:57:52PM +0100, Daniel Vetter wrote:
> On Fri, Mar 17, 2017 at 01:08:43PM +0200, Ville Syrjälä wrote:
> > On Fri, Mar 17, 2017 at 03:46:34PM +0530, Shirish S wrote:
> > > On Fri, Mar 17, 2017 at 3:26 PM, Ville Syrjälä
> > > wrote:
> > > > On Fri, Mar 17, 2017 at 01:25:08PM +0530, Shirish S wrote:
> > > >> update_plane() and disable_plane() functions
> > > >> assoiciated with setting plane are called
> > > >> without any check, causing kernel panic.
> > > >
> > > > Why are you registering a plane without the funcs?
> > > >
> > > Basically, enabling planes and making them fully functional is
> > > generally a 2 -step process,
> > > so i suggest for new drivers wanting to implement/re-design planes,
> > > would like to tap
> > > the flow at enabling(listing caps) and later at ensuring it works.
> >
> > I don't think there's much point in exposing something that
> > doesn't work. And even if you do, you could always just use
> > stub functions.
>
> Yes, just wire up stub functions if you want to enable planes with
> multi-step patch series.
>
> > > I noticed that there is a underlying assumption only for
> > > plane->(funcs) are implemented, whereas for
> > > other function for crtc/connector/encoder function calls there is a
> > > sanity check(or WARN_ON) through out the framework.
> > >
> > > I believe this check wont cause any performance/functional impact.
> > > Please let me know if am missing anything.
> > > And further more help developers to focus on enabling planes via
> > > various tests without causing reboots/system hangs.
> >
> > I don't particularly like adding more unconditional runtime checks
> > that just to protect developers from themselves. If you really
> > think there's value in these, then at least add the checks into
> > the plane init codepath so that it's a one time cost.
> >
> > The same approach could be used for all the other non-optional
> > hooks. Otherwise the same WARN_ON()s would have to be sprinkled
> > all over the place, and there's always the risk of missing a few
> > codepaths that call a specific hook.
>
> I think for these here there's negative value - it allows developers to
> create completely broken planes. Stub functions really seem like a much
> better idea.
I was thinking
drm_whatever_init()
{
if (WARN_ON(!funcs->mandatory_thing))
return -EINVAL;
}
rather than putting the WARN_ON()s around each call of
funcs->mandatory_thing().
That will fail gracefully (which I guess is what people are after here),
and gives the developer a clear message what's missing.
--
Ville Syrjälä
Intel OTC
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/vc4: use platform_register_drivers
Use platform_register_drivers instead of open coding the iteration over
component platform drivers in the vc4_drv module.
Signed-off-by: Philipp Zabel
---
drivers/gpu/drm/vc4/vc4_drv.c | 22 --
1 file changed, 8 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
index 205c1961ffb4c..61e674baf3a6f 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.c
+++ b/drivers/gpu/drm/vc4/vc4_drv.c
@@ -349,26 +349,20 @@ static struct platform_driver vc4_platform_driver = {
static int __init vc4_drm_register(void)
{
- int i, ret;
+ int ret;
+
+ ret = platform_register_drivers(component_drivers,
+ ARRAY_SIZE(component_drivers));
+ if (ret)
+ return ret;
- for (i = 0; i < ARRAY_SIZE(component_drivers); i++) {
- ret = platform_driver_register(component_drivers[i]);
- if (ret) {
- while (--i >= 0)
-
platform_driver_unregister(component_drivers[i]);
- return ret;
- }
- }
return platform_driver_register(&vc4_platform_driver);
}
static void __exit vc4_drm_unregister(void)
{
- int i;
-
- for (i = ARRAY_SIZE(component_drivers) - 1; i >= 0; i--)
- platform_driver_unregister(component_drivers[i]);
-
+ platform_unregister_drivers(component_drivers,
+ ARRAY_SIZE(component_drivers));
platform_driver_unregister(&vc4_platform_driver);
}
--
2.11.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/mediatek: use platform_register_drivers
Use platform_register_drivers instead of open coding the iteration over
component platform drivers in the mtk_drm_drv and mtk_hdmi modules.
Signed-off-by: Philipp Zabel
---
drivers/gpu/drm/mediatek/mtk_drm_drv.c | 27 ---
drivers/gpu/drm/mediatek/mtk_hdmi.c| 27 ---
2 files changed, 8 insertions(+), 46 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index f5a1fd9b3ecc7..20e540aa9e87a 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -536,33 +536,14 @@ static struct platform_driver * const mtk_drm_drivers[] =
{
static int __init mtk_drm_init(void)
{
- int ret;
- int i;
-
- for (i = 0; i < ARRAY_SIZE(mtk_drm_drivers); i++) {
- ret = platform_driver_register(mtk_drm_drivers[i]);
- if (ret < 0) {
- pr_err("Failed to register %s driver: %d\n",
- mtk_drm_drivers[i]->driver.name, ret);
- goto err;
- }
- }
-
- return 0;
-
-err:
- while (--i >= 0)
- platform_driver_unregister(mtk_drm_drivers[i]);
-
- return ret;
+ return platform_register_drivers(mtk_drm_drivers,
+ARRAY_SIZE(mtk_drm_drivers));
}
static void __exit mtk_drm_exit(void)
{
- int i;
-
- for (i = ARRAY_SIZE(mtk_drm_drivers) - 1; i >= 0; i--)
- platform_driver_unregister(mtk_drm_drivers[i]);
+ platform_unregister_drivers(mtk_drm_drivers,
+ ARRAY_SIZE(mtk_drm_drivers));
}
module_init(mtk_drm_init);
diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c
b/drivers/gpu/drm/mediatek/mtk_hdmi.c
index 4e2f9c43395c2..65ac296a02fac 100644
--- a/drivers/gpu/drm/mediatek/mtk_hdmi.c
+++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c
@@ -1798,33 +1798,14 @@ static struct platform_driver * const
mtk_hdmi_drivers[] = {
static int __init mtk_hdmitx_init(void)
{
- int ret;
- int i;
-
- for (i = 0; i < ARRAY_SIZE(mtk_hdmi_drivers); i++) {
- ret = platform_driver_register(mtk_hdmi_drivers[i]);
- if (ret < 0) {
- pr_err("Failed to register %s driver: %d\n",
- mtk_hdmi_drivers[i]->driver.name, ret);
- goto err;
- }
- }
-
- return 0;
-
-err:
- while (--i >= 0)
- platform_driver_unregister(mtk_hdmi_drivers[i]);
-
- return ret;
+ return platform_register_drivers(mtk_hdmi_drivers,
+ARRAY_SIZE(mtk_hdmi_drivers));
}
static void __exit mtk_hdmitx_exit(void)
{
- int i;
-
- for (i = ARRAY_SIZE(mtk_hdmi_drivers) - 1; i >= 0; i--)
- platform_driver_unregister(mtk_hdmi_drivers[i]);
+ platform_unregister_drivers(mtk_hdmi_drivers,
+ ARRAY_SIZE(mtk_hdmi_drivers));
}
module_init(mtk_hdmitx_init);
--
2.11.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: add check for plane functions
On Fri, Mar 17, 2017 at 01:08:43PM +0200, Ville Syrjälä wrote:
> On Fri, Mar 17, 2017 at 03:46:34PM +0530, Shirish S wrote:
> > On Fri, Mar 17, 2017 at 3:26 PM, Ville Syrjälä
> > wrote:
> > > On Fri, Mar 17, 2017 at 01:25:08PM +0530, Shirish S wrote:
> > >> update_plane() and disable_plane() functions
> > >> assoiciated with setting plane are called
> > >> without any check, causing kernel panic.
> > >
> > > Why are you registering a plane without the funcs?
> > >
> > Basically, enabling planes and making them fully functional is
> > generally a 2 -step process,
> > so i suggest for new drivers wanting to implement/re-design planes,
> > would like to tap
> > the flow at enabling(listing caps) and later at ensuring it works.
>
> I don't think there's much point in exposing something that
> doesn't work. And even if you do, you could always just use
> stub functions.
Yes, just wire up stub functions if you want to enable planes with
multi-step patch series.
> > I noticed that there is a underlying assumption only for
> > plane->(funcs) are implemented, whereas for
> > other function for crtc/connector/encoder function calls there is a
> > sanity check(or WARN_ON) through out the framework.
> >
> > I believe this check wont cause any performance/functional impact.
> > Please let me know if am missing anything.
> > And further more help developers to focus on enabling planes via
> > various tests without causing reboots/system hangs.
>
> I don't particularly like adding more unconditional runtime checks
> that just to protect developers from themselves. If you really
> think there's value in these, then at least add the checks into
> the plane init codepath so that it's a one time cost.
>
> The same approach could be used for all the other non-optional
> hooks. Otherwise the same WARN_ON()s would have to be sprinkled
> all over the place, and there's always the risk of missing a few
> codepaths that call a specific hook.
I think for these here there's negative value - it allows developers to
create completely broken planes. Stub functions really seem like a much
better idea.
Note that for the helpers we allow you to not have a lot of the hooks,
since there's indeed piles of hw where they are not needed. But this one
here really is.
Also, with an atomic driver you get it for free, so there's really never
any reason at all not too hook this up.
Add this to the list of reasons why you _really_ should have an atomic
driver :-)
-Daniel
>
> > >>
> > >> This patch adds the required check to avoid it.
> > >>
> > >> Change-Id: I0d6792608b33e674c217388aa57c4b7d680d9bc7
> > >> Signed-off-by: Shirish S
> > >> ---
> > >> drivers/gpu/drm/drm_plane.c | 6 ++
> > >> 1 file changed, 6 insertions(+)
> > >>
> > >> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> > >> index 249c0ae..f675f8b 100644
> > >> --- a/drivers/gpu/drm/drm_plane.c
> > >> +++ b/drivers/gpu/drm/drm_plane.c
> > >> @@ -456,6 +456,12 @@ static int __setplane_internal(struct drm_plane
> > >> *plane,
> > >> {
> > >> int ret = 0;
> > >>
> > >> + if (plane->funcs->disable_plane == NULL ||
> > >> + plane->funcs->update_plane == NULL) {
> > >> + DRM_ERROR("plane funcs not implemented\n");
> > >> + ret = -EPERM;
> > >> + goto out;
> > >> + }
> > >> /* No fb means shut it down */
> > >> if (!fb) {
> > >> plane->old_fb = plane->fb;
> > >> --
> > >> 2.7.4
> > >>
> > >> ___
> > >> dri-devel mailing list
> > >> dri-devel@lists.freedesktop.org
> > >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > >
> > > --
> > > Ville Syrjälä
> > > Intel OTC
>
> --
> Ville Syrjälä
> Intel OTC
> ___
> 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/atomic: protect crtc|connector->state with rcu
Op 16-03-17 om 21:15 schreef Daniel Vetter:
> On Thu, Mar 16, 2017 at 5:09 PM, Maarten Lankhorst
> wrote:
>> Op 16-03-17 om 16:52 schreef Daniel Vetter:
>>> The vblank code really wants to look at crtc->state without having to
>>> take a ww_mutex. One option might be to take one of the vblank locks
>>> right when assigning crtc->state, which would ensure that the vblank
>>> code doesn't race and access freed memory.
>> I'm not sure this is the right approach for vblank.
> It's not, it's just that I've had to resurrect this patch quickly
> before leaving and accidentally left the vblank stuff in.
>
>> crtc->state might not be the same as the current state in case of a
>> nonblocking
>> modeset that hasn't even disabled the old crtc yet.
>>> But userspace tends to poke the vblank_ioctl to query the current
>>> vblank timestamp rather often, and going all in with rcu would help a
>>> bit. We're not there yet, but also doesn't really hurt.
>>>
>>> v2: Maarten needs this to make connector properties atomic, so he can
>>> peek at state from the various ->mode_valid hooks.
>>>
>>> Cc: Maarten Lankhorst
>>> Signed-off-by: Daniel Vetter
>>> ---
>>> drivers/gpu/drm/drm_atomic.c| 26 +-
>>> drivers/gpu/drm/drm_atomic_helper.c | 2 +-
>>> include/drm/drm_atomic.h| 5 +
>>> include/drm/drm_connector.h | 13 -
>>> include/drm/drm_crtc.h | 9 -
>>> 5 files changed, 43 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
>>> index 9b892af7811a..ba11e31ff9ba 100644
>>> --- a/drivers/gpu/drm/drm_atomic.c
>>> +++ b/drivers/gpu/drm/drm_atomic.c
>>> @@ -213,16 +213,10 @@ void drm_atomic_state_clear(struct drm_atomic_state
>>> *state)
>>> }
>>> EXPORT_SYMBOL(drm_atomic_state_clear);
>>>
>>> -/**
>>> - * __drm_atomic_state_free - free all memory for an atomic state
>>> - * @ref: This atomic state to deallocate
>>> - *
>>> - * This frees all memory associated with an atomic state, including all the
>>> - * per-object state for planes, crtcs and connectors.
>>> - */
>>> -void __drm_atomic_state_free(struct kref *ref)
>>> +void ___drm_atomic_state_free(struct rcu_head *rhead)
>>> {
>>> - struct drm_atomic_state *state = container_of(ref, typeof(*state),
>>> ref);
>>> + struct drm_atomic_state *state =
>>> + container_of(rhead, typeof(*state), rhead);
>>> struct drm_mode_config *config = &state->dev->mode_config;
>>>
>>> drm_atomic_state_clear(state);
>>> @@ -236,6 +230,20 @@ void __drm_atomic_state_free(struct kref *ref)
>>> kfree(state);
>>> }
>>> }
>> whatisRCU.txt:
>> "This function invokes func(head) after a grace period has elapsed.
>> This invocation might happen from either softirq or process context,
>> so the function is not permitted to block."
>>
>> Looking at
>> commit 6f0f02dc56f18760b46dc1bf5b3f7386869d4162
>> Author: Chris Wilson
>> Date: Mon Jan 23 21:29:39 2017 +
>>
>> drm/i915: Move atomic state free from out of fence release
>>
>> I would say that drm_atomic_state_free would definitely block..
>>
>> The only thing that makes sense IMO is doing kfree_rcu on the object_states.
>> But I don't think RCU is the answer here, it won't protect you against using
>> the wrong crtc state.
>>
>> I think I would try to use the crtc ww_mutex in the vblank code and
>> serialize it to pending commits, if at all possible.
> Oops. I guess it should have come with "totally untested". In that
> case we need a workter which does a synchronize_rcu() before
> releasing. I don't just want to do a simple kfree_rcu() because by
> that point we've (partially) destroyed the state alreayd (so it's
> already unsafe to access, and special ruels are ugly). And doing it
> here before we release anything in the core would avoid the need for
> drivers to bother with kfree_rcu().
>
> I'll try to respin something less obviously buggy tomorrow :-)
It will still be buggy tomorrow, since you have no way to know if the current
hardware crtc_state is anything like crtc->state.
:(
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 1/4] dma-fence: add dma_fence_match_context helper
Add a helper to check if all fences in a fence array are from a given
context. For convenience, the function can also handle being given a
non-array fence.
Signed-off-by: Philipp Zabel
---
drivers/dma-buf/dma-fence-array.c | 26 ++
include/linux/dma-fence-array.h | 2 ++
2 files changed, 28 insertions(+)
diff --git a/drivers/dma-buf/dma-fence-array.c
b/drivers/dma-buf/dma-fence-array.c
index 67eb7c8fb88c3..0350829ba62e7 100644
--- a/drivers/dma-buf/dma-fence-array.c
+++ b/drivers/dma-buf/dma-fence-array.c
@@ -144,3 +144,29 @@ struct dma_fence_array *dma_fence_array_create(int
num_fences,
return array;
}
EXPORT_SYMBOL(dma_fence_array_create);
+
+/**
+ * dma_fence_match_context - Check if all fences are from the given context
+ * @fence: [in]fence or fence array
+ * @context: [in]fence context to check all fences against
+ *
+ * Checks the provided fence or, for a fence array, all fences in the array
+ * against the given context. Returns false if any fence is from a different
+ * context.
+ */
+bool dma_fence_match_context(struct dma_fence *fence, u64 context)
+{
+ struct dma_fence_array *array = to_dma_fence_array(fence);
+ unsigned i;
+
+ if (!dma_fence_is_array(fence))
+ return fence->context == context;
+
+ for (i = 0; i < array->num_fences; i++) {
+ if (array->fences[i]->context != context)
+ return false;
+ }
+
+ return true;
+}
+EXPORT_SYMBOL(dma_fence_match_context);
diff --git a/include/linux/dma-fence-array.h b/include/linux/dma-fence-array.h
index 5900945f962dd..332a5420243c4 100644
--- a/include/linux/dma-fence-array.h
+++ b/include/linux/dma-fence-array.h
@@ -83,4 +83,6 @@ struct dma_fence_array *dma_fence_array_create(int num_fences,
u64 context, unsigned seqno,
bool signal_on_any);
+bool dma_fence_match_context(struct dma_fence *fence, u64 context);
+
#endif /* __LINUX_DMA_FENCE_ARRAY_H */
--
2.11.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 4/4] drm/etnaviv: submit support for out-fences
Based on commit 4cd0945901a6 ("drm/msm: submit support for out-fences").
We increment the minor driver version so userspace can detect explicit
fence support.
Signed-off-by: Philipp Zabel
Reviewed-by: Gustavo Padovan
---
v2: Dropped unnecessary braces
---
drivers/gpu/drm/etnaviv/etnaviv_drv.c| 2 +-
drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 26 ++
include/uapi/drm/etnaviv_drm.h | 6 --
3 files changed, 31 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
index 289a9f8c66710..5255278dde560 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
@@ -512,7 +512,7 @@ static struct drm_driver etnaviv_drm_driver = {
.desc = "etnaviv DRM",
.date = "20151214",
.major = 1,
- .minor = 0,
+ .minor = 1,
};
/*
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
index be88411de35a4..8a1381ca0354b 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
@@ -308,6 +308,8 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void
*data,
struct etnaviv_cmdbuf *cmdbuf;
struct etnaviv_gpu *gpu;
struct dma_fence *in_fence = NULL;
+ struct sync_file *sync_file = NULL;
+ int out_fence_fd = -1;
void *stream;
int ret;
@@ -375,6 +377,14 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void
*data,
goto err_submit_cmds;
}
+ if (args->flags & ETNA_SUBMIT_FENCE_FD_OUT) {
+ out_fence_fd = get_unused_fd_flags(O_CLOEXEC);
+ if (out_fence_fd < 0) {
+ ret = out_fence_fd;
+ goto err_submit_cmds;
+ }
+ }
+
submit = submit_create(dev, gpu, args->nr_bos);
if (!submit) {
ret = -ENOMEM;
@@ -437,10 +447,24 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void
*data,
goto out;
}
+ if (args->flags & ETNA_SUBMIT_FENCE_FD_OUT) {
+ sync_file = sync_file_create(submit->fence);
+ if (!sync_file) {
+ dma_fence_put(submit->fence);
+ submit->fence = NULL;
+ ret = -ENOMEM;
+ goto out;
+ }
+ }
+
ret = etnaviv_gpu_submit(gpu, submit, cmdbuf);
if (ret == 0)
cmdbuf = NULL;
+ if (args->flags & ETNA_SUBMIT_FENCE_FD_OUT)
+ fd_install(out_fence_fd, sync_file->file);
+
+ args->fence_fd = out_fence_fd;
args->fence = submit->fence->seqno;
out:
@@ -460,6 +484,8 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void
*data,
submit_cleanup(submit);
err_submit_cmds:
+ if (ret && (out_fence_fd >= 0))
+ put_unused_fd(out_fence_fd);
/* if we still own the cmdbuf */
if (cmdbuf)
etnaviv_cmdbuf_free(cmdbuf);
diff --git a/include/uapi/drm/etnaviv_drm.h b/include/uapi/drm/etnaviv_drm.h
index e9c388a1d8ebe..76f6f78a352ba 100644
--- a/include/uapi/drm/etnaviv_drm.h
+++ b/include/uapi/drm/etnaviv_drm.h
@@ -156,8 +156,10 @@ struct drm_etnaviv_gem_submit_bo {
*/
#define ETNA_SUBMIT_NO_IMPLICIT 0x0001
#define ETNA_SUBMIT_FENCE_FD_IN 0x0002
+#define ETNA_SUBMIT_FENCE_FD_OUT0x0004
#define ETNA_SUBMIT_FLAGS (ETNA_SUBMIT_NO_IMPLICIT | \
-ETNA_SUBMIT_FENCE_FD_IN)
+ETNA_SUBMIT_FENCE_FD_IN | \
+ETNA_SUBMIT_FENCE_FD_OUT)
#define ETNA_PIPE_3D 0x00
#define ETNA_PIPE_2D 0x01
#define ETNA_PIPE_VG 0x02
@@ -172,7 +174,7 @@ struct drm_etnaviv_gem_submit {
__u64 relocs; /* in, ptr to array of submit_reloc's */
__u64 stream; /* in, ptr to cmdstream */
__u32 flags; /* in, mask of ETNA_SUBMIT_x */
- __s32 fence_fd; /* in, fence fd (see ETNA_SUBMIT_FENCE_FD_IN) */
+ __s32 fence_fd; /* in/out, fence fd (see ETNA_SUBMIT_FENCE_FD_x)
*/
};
/* The normal way to synchronize with the GPU is just to CPU_PREP on
--
2.11.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 2/4] drm/etnaviv: submit support for in-fences
Loosely based on commit f0a42bb5423a ("drm/msm: submit support for
in-fences"). Unfortunately, struct drm_etnaviv_gem_submit doesn't have
a flags field yet, so we have to extend the structure and trust that
drm_ioctl will clear the flags for us if an older userspace only submits
part of the struct.
Signed-off-by: Philipp Zabel
---
v2: Use dma_fence_match_context to skip waiting if all in-fences are ours.
---
drivers/gpu/drm/etnaviv/Kconfig | 1 +
drivers/gpu/drm/etnaviv/etnaviv_gem.h| 1 +
drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 34 +++-
drivers/gpu/drm/etnaviv/etnaviv_gpu.c| 5 +++-
drivers/gpu/drm/etnaviv/etnaviv_gpu.h| 2 +-
include/uapi/drm/etnaviv_drm.h | 6 +
6 files changed, 46 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/etnaviv/Kconfig b/drivers/gpu/drm/etnaviv/Kconfig
index cc1731c5289c2..71cee4e9fefbb 100644
--- a/drivers/gpu/drm/etnaviv/Kconfig
+++ b/drivers/gpu/drm/etnaviv/Kconfig
@@ -5,6 +5,7 @@ config DRM_ETNAVIV
depends on ARCH_MXC || ARCH_DOVE || (ARM && COMPILE_TEST)
depends on MMU
select SHMEM
+ select SYNC_FILE
select TMPFS
select IOMMU_API
select IOMMU_SUPPORT
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.h
b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
index e63ff116a3b3d..120410d67eb5b 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
@@ -107,6 +107,7 @@ struct etnaviv_gem_submit {
u32 fence;
unsigned int nr_bos;
struct etnaviv_gem_submit_bo bos[0];
+ u32 flags;
};
int etnaviv_gem_wait_bo(struct etnaviv_gpu *gpu, struct drm_gem_object *obj,
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
index 726090d7a6ace..fb8d5befbf4f1 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
@@ -14,7 +14,9 @@
* this program. If not, see
[PATCH v2 3/4] drm/etnaviv: move fence allocation out of etnaviv_gpu_submit()
The next patch will need the dma_fence to create the sync_file in
etnaviv_ioctl_gem_submit, in case an out_fence_fd is requested.
Signed-off-by: Philipp Zabel
Reviewed-by: Gustavo Padovan
---
drivers/gpu/drm/etnaviv/etnaviv_gem.h| 3 ++-
drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 8 +++-
drivers/gpu/drm/etnaviv/etnaviv_gpu.c| 21 ++---
drivers/gpu/drm/etnaviv/etnaviv_gpu.h| 1 +
4 files changed, 16 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.h
b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
index 120410d67eb5b..c4a091e874269 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
@@ -20,6 +20,7 @@
#include
#include "etnaviv_drv.h"
+struct dma_fence;
struct etnaviv_gem_ops;
struct etnaviv_gem_object;
@@ -104,7 +105,7 @@ struct etnaviv_gem_submit {
struct drm_device *dev;
struct etnaviv_gpu *gpu;
struct ww_acquire_ctx ticket;
- u32 fence;
+ struct dma_fence *fence;
unsigned int nr_bos;
struct etnaviv_gem_submit_bo bos[0];
u32 flags;
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
index fb8d5befbf4f1..be88411de35a4 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
@@ -431,11 +431,17 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void
*data,
memcpy(cmdbuf->vaddr, stream, args->stream_size);
cmdbuf->user_size = ALIGN(args->stream_size, 8);
+ submit->fence = etnaviv_gpu_fence_alloc(gpu);
+ if (!submit->fence) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
ret = etnaviv_gpu_submit(gpu, submit, cmdbuf);
if (ret == 0)
cmdbuf = NULL;
- args->fence = submit->fence;
+ args->fence = submit->fence->seqno;
out:
submit_unpin_objects(submit);
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
index 51d52c72aef17..a439700cc577f 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
@@ -1047,7 +1047,7 @@ static const struct dma_fence_ops etnaviv_fence_ops = {
.release = etnaviv_fence_release,
};
-static struct dma_fence *etnaviv_gpu_fence_alloc(struct etnaviv_gpu *gpu)
+struct dma_fence *etnaviv_gpu_fence_alloc(struct etnaviv_gpu *gpu)
{
struct etnaviv_fence *f;
@@ -1290,7 +1290,6 @@ void etnaviv_gpu_pm_put(struct etnaviv_gpu *gpu)
int etnaviv_gpu_submit(struct etnaviv_gpu *gpu,
struct etnaviv_gem_submit *submit, struct etnaviv_cmdbuf *cmdbuf)
{
- struct dma_fence *fence;
unsigned int event, i;
int ret;
@@ -1314,18 +1313,10 @@ int etnaviv_gpu_submit(struct etnaviv_gpu *gpu,
goto out_pm_put;
}
- fence = etnaviv_gpu_fence_alloc(gpu);
- if (!fence) {
- event_free(gpu, event);
- ret = -ENOMEM;
- goto out_pm_put;
- }
-
mutex_lock(&gpu->lock);
- gpu->event[event].fence = fence;
- submit->fence = fence->seqno;
- gpu->active_fence = submit->fence;
+ gpu->event[event].fence = submit->fence;
+ gpu->active_fence = submit->fence->seqno;
if (gpu->lastctx != cmdbuf->ctx) {
gpu->mmu->need_flush = true;
@@ -1335,7 +1326,7 @@ int etnaviv_gpu_submit(struct etnaviv_gpu *gpu,
etnaviv_buffer_queue(gpu, event, cmdbuf);
- cmdbuf->fence = fence;
+ cmdbuf->fence = submit->fence;
list_add_tail(&cmdbuf->node, &gpu->active_cmd_list);
/* We're committed to adding this command buffer, hold a PM reference */
@@ -1351,10 +1342,10 @@ int etnaviv_gpu_submit(struct etnaviv_gpu *gpu,
if (submit->bos[i].flags & ETNA_SUBMIT_BO_WRITE)
reservation_object_add_excl_fence(etnaviv_obj->resv,
- fence);
+ submit->fence);
else
reservation_object_add_shared_fence(etnaviv_obj->resv,
- fence);
+ submit->fence);
}
cmdbuf->nr_bos = submit->nr_bos;
hangcheck_timer_reset(gpu);
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
index dc27c7a039060..20f7191018daf 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
@@ -180,6 +180,7 @@ int etnaviv_gpu_init(struct etnaviv_gpu *gpu);
int etnaviv_gpu_debugfs(struct etnaviv_gpu *gpu, struct seq_file *m);
#endif
+struct dma_fence *etnaviv_gpu_fence_alloc(struct etnaviv_gpu *gpu);
int etnaviv_gpu_fence_sync_obj(struct etnaviv_gem_object *etnaviv_obj,
unsigned int cont
Re: [PATCH v3 3/6] documentation: media: Add documentation for new RGB and YUV bus formats
On 03/16/2017 06:01 PM, Archit Taneja wrote: > > > On 3/7/2017 10:12 PM, Neil Armstrong wrote: >> Add documentation for added Bus Formats to describe RGB and YUS formats used > > s/YUS/YUV Thanks again > >> as input to the Synopsys DesignWare HDMI TX Controller. >> >> Signed-off-by: Neil Armstrong >> --- >> Documentation/media/uapi/v4l/subdev-formats.rst | 4992 >> ++- >> 1 file changed, 3963 insertions(+), 1029 deletions(-) > > Do we know if there is a better way to add more columns without > adding so many lines? It seems not, the reason is written in the commands. > If not, one option could be to create a separate tables for > 48 bit RGB formats, 48 bit YUV formats etc. It would be simple indeed, any V4L guys for an advice here ? Thanks, Neil > > > > Thanks, > Archit > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 2/6] media: uapi: Add RGB and YUV bus formats for Synopsys HDMI TX Controller
On 03/16/2017 06:06 PM, Archit Taneja wrote: > > > On 3/7/2017 10:12 PM, Neil Armstrong wrote: >> In order to describe the RGB and YUB bus formats used to feed the > > s/YUB/YUV Thanks for pointing me this ! Neil > >> Synopsys DesignWare HDMI TX Controller, add missing formats to the >> list of Bus Formats. >> >> Documentation for these formats is added in a separate patch. >> > > Reviewed-by: Archit Taneja > >> Signed-off-by: Neil Armstrong >> --- >> include/uapi/linux/media-bus-format.h | 13 +++-- >> 1 file changed, 11 insertions(+), 2 deletions(-) >> >> diff --git a/include/uapi/linux/media-bus-format.h >> b/include/uapi/linux/media-bus-format.h >> index 2168759..7cc820b 100644 >> --- a/include/uapi/linux/media-bus-format.h >> +++ b/include/uapi/linux/media-bus-format.h >> @@ -33,7 +33,7 @@ >> >> #define MEDIA_BUS_FMT_FIXED0x0001 >> >> -/* RGB - next is0x1018 */ >> +/* RGB - next is0x101b */ >> #define MEDIA_BUS_FMT_RGB444_1X120x1016 >> #define MEDIA_BUS_FMT_RGB444_2X8_PADHI_BE0x1001 >> #define MEDIA_BUS_FMT_RGB444_2X8_PADHI_LE0x1002 >> @@ -57,8 +57,11 @@ >> #define MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA0x1012 >> #define MEDIA_BUS_FMT_ARGB_1X320x100d >> #define MEDIA_BUS_FMT_RGB888_1X32_PADHI0x100f >> +#define MEDIA_BUS_FMT_RGB101010_1X300x1018 >> +#define MEDIA_BUS_FMT_RGB121212_1X360x1019 >> +#define MEDIA_BUS_FMT_RGB161616_1X480x101a >> >> -/* YUV (including grey) - next is0x2026 */ >> +/* YUV (including grey) - next is0x202c */ >> #define MEDIA_BUS_FMT_Y8_1X80x2001 >> #define MEDIA_BUS_FMT_UV8_1X80x2015 >> #define MEDIA_BUS_FMT_UYVY8_1_5X80x2002 >> @@ -90,12 +93,18 @@ >> #define MEDIA_BUS_FMT_YVYU10_1X200x200e >> #define MEDIA_BUS_FMT_VUY8_1X240x2024 >> #define MEDIA_BUS_FMT_YUV8_1X240x2025 >> +#define MEDIA_BUS_FMT_UYVY8_1_1X240x2026 >> #define MEDIA_BUS_FMT_UYVY12_1X240x2020 >> #define MEDIA_BUS_FMT_VYUY12_1X240x2021 >> #define MEDIA_BUS_FMT_YUYV12_1X240x2022 >> #define MEDIA_BUS_FMT_YVYU12_1X240x2023 >> #define MEDIA_BUS_FMT_YUV10_1X300x2016 >> +#define MEDIA_BUS_FMT_UYVY10_1_1X300x2027 >> #define MEDIA_BUS_FMT_AYUV8_1X320x2017 >> +#define MEDIA_BUS_FMT_UYVY12_1_1X360x2028 >> +#define MEDIA_BUS_FMT_YUV12_1X360x2029 >> +#define MEDIA_BUS_FMT_YUV16_1X480x202a >> +#define MEDIA_BUS_FMT_UYVY16_1_1X480x202b >> >> /* Bayer - next is0x3021 */ >> #define MEDIA_BUS_FMT_SBGGR8_1X80x3001 >> > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 100258] [BAT][SKL] Serial console hung in kms_flip when running IGT
https://bugs.freedesktop.org/show_bug.cgi?id=100258 Tomi Sarvela changed: What|Removed |Added OS|All |Linux (All) QA Contact||intel-gfx-bugs@lists.freede ||sktop.org CC||intel-gfx-bugs@lists.freede ||sktop.org Component|IGT |DRM/Intel Hardware|Other |x86-64 (AMD64) i915 platform||SKL Assignee|dri-devel@lists.freedesktop |intel-gfx-bugs@lists.freede |.org|sktop.org -- 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 100258] [BAT][SKL] Serial console hung in kms_flip when running IGT
https://bugs.freedesktop.org/show_bug.cgi?id=100258 Bug ID: 100258 Summary: [BAT][SKL] Serial console hung in kms_flip when running IGT Product: DRI Version: DRI git Hardware: Other OS: All Status: NEW Severity: normal Priority: medium Component: IGT Assignee: dri-devel@lists.freedesktop.org Reporter: tomi.p.sarv...@intel.com Testhost is i7-6700K on Gigabyte Z170 MB. Serial is real COM1 header. Lockup happened while running IGT testlist in loop (started from serial console, if that matters). Kernel is built from DRM-Tip today. BIOS newest. Kconfig at https://intel-gfx-ci.01.org/CI/CI_DRM_2352/kernel.config.bz2 [22018.856538] [drm:intel_atomic_commit_tail [i915]] [CRTC:46:pipe C] [22018.856551] [drm:intel_power_well_disable [i915]] disabling DC off [22018.856566] [drm:skl_enable_dc6 [i915]] Enabling DC6 [22018.856577] [drm:gen9_set_dc_state [i915]] Setting DC state from 00 to 02 [22018.856997] [drm:intel_power_well_disable [i915]] disabling always-on [22018.857038] [drm:intel_runtime_suspend [i915]] Suspending device [22018.857680] [IGT] kms_flip: exiting, ret=0 [22018.859602] [drm:intel_runtime_suspend [i915]] Device suspended [22018.884658] [drm:intel_runtime_resume [i915]] Resuming device [22018.888911] [drm:intel_runtime_resume [i915]] Device resumed [22018.888993] [drm:intel_runtime_suspend [i915]] Suspending device [22018.891615] [drm:intel_runtime_suspend [i915]] Device suspended [22055.545628] INFO: task kms_flip:25920 blocked for more than 30 seconds. [22055.545629] Tainted: G U W 4.11.0-rc2-CI-CI_DRM_2357+ #1 [22055.545629] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. [22055.545630] kms_flipD0 25920 20731 0x [22055.545631] Call Trace: [22055.545635] __schedule+0x3fd/0xbc0 [22055.545636] ? __mutex_unlock_slowpath+0x3e/0x2e0 [22055.545637] schedule+0x3b/0x90 [22055.545639] schedule_timeout+0x23b/0x490 [22055.545641] ? __save_stack_trace+0x7e/0xd0 [22055.545643] __down+0x80/0xc0 [22055.545644] ? _raw_spin_lock_irqsave+0x49/0x60 [22055.545645] down+0x3c/0x50 [22055.545647] console_lock+0x27/0x70 [22055.545648] ? capable+0x17/0x20 [22055.545650] vt_ioctl+0x51a/0x1440 [22055.545651] ? _raw_spin_unlock_irqrestore+0x3d/0x60 [22055.545652] ? __slab_free+0x39f/0x4f0 [22055.545654] tty_ioctl+0x384/0xe80 [22055.545655] ? _raw_spin_unlock_irqrestore+0x3d/0x60 [22055.545656] ? debug_check_no_obj_freed+0x117/0x220 [22055.545658] do_vfs_ioctl+0x90/0x6e0 [22055.545659] ? putname+0x4f/0x60 [22055.545661] SyS_ioctl+0x3c/0x70 [22055.545662] entry_SYSCALL_64_fastpath+0x1c/0xb1 [22055.545663] RIP: 0033:0x7f6c17851357 [22055.545663] RSP: 002b:7fffa5d50198 EFLAGS: 0246 ORIG_RAX: 0010 [22055.545664] RAX: ffda RBX: 01566ea0 RCX: 7f6c17851357 [22055.545664] RDX: RSI: 4b3a RDI: 0006 [22055.545665] RBP: R08: 7fffa5d50160 R09: [22055.545665] R10: 0008 R11: 0246 R12: [22055.545665] R13: 7fffa5d50ed0 R14: R15: [22055.545667] INFO: lockdep is turned off. [22055.545667] NMI backtrace for cpu 5 [22055.545669] CPU: 5 PID: 66 Comm: khungtaskd Tainted: G U W 4.11.0-rc2-CI-CI_DRM_2357+ #1 [22055.545669] Hardware name: Gigabyte Technology Co., Ltd. Z170X-UD5/Z170X-UD5-CF, BIOS F21 01/06/2 017 [22055.545669] Call Trace: [22055.545671] dump_stack+0x67/0x92 [22055.545672] ? irq_force_complete_move+0xf0/0xf0 [22055.545673] nmi_cpu_backtrace+0x92/0xa0 [22055.545674] ? irq_force_complete_move+0xf0/0xf0 [22055.545675] nmi_trigger_cpumask_backtrace+0xc0/0x100 [22055.545677] arch_trigger_cpumask_backtrace+0x14/0x20 [22055.545678] watchdog+0x433/0x600 [22055.545678] ? watchdog+0x9c/0x600 [22055.545680] kthread+0x107/0x140 [22055.545680] ? reset_hung_task_detector+0x10/0x10 [22055.545681] ? kthread_create_on_node+0x40/0x40 [22055.545682] ret_from_fork+0x2e/0x40 [22055.545684] Sending NMI from CPU 5 to CPUs 0-4,6-7: [22055.545690] NMI backtrace for cpu 0 skipped: idling at pc 0x818760f0 [22055.545691] NMI backtrace for cpu 1 skipped: idling at pc 0x818760f0 [22055.545721] NMI backtrace for cpu 7 skipped: idling at pc 0x818760f0 [22055.545722] NMI backtrace for cpu 3 skipped: idling at pc 0x818760f0 [22055.545723] NMI backtrace for cpu 2 skipped: idling at pc 0x818760f0 [22055.545723] NMI backtrace for cpu 6 skipped: idling at pc 0x818760f0 [22055.546688] NMI backtrace for cpu 4 [22055.546688] CPU: 4 PID: 25851 Comm: kworker/4:1 Tainted: G U W 4.11.0-rc2-CI-CI_DRM_23 57+ #1 [22055.546689] Hardware name: Gigabyte Technology Co., Ltd. Z170X-UD5/Z170X-UD5-CF, BIOS F21 01/06/2 017 [22055.546689] Workqueue: usb_hub_wq hub_event [22055.
Re: [PATCHv2 08/10] drm: omapdrm: crtc: handle framedone directly
> @@ -264,8 +267,10 @@ int omap_drm_irq_install(struct drm_device *dev)
> priv->irq_mask |= omap_underflow_irqs[i];
> }
>
> - for (i = 0; i < num_mgrs; ++i)
> + for (i = 0; i < num_mgrs; ++i) {
> priv->irq_mask |= dispc_mgr_get_sync_lost_irq(i);
> + priv->irq_mask |= dispc_mgr_get_framedone_irq(i);
> + }
>
i++, we are no C++. OTOH, not your fault.
Acked-by: Pavel Machek
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures)
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
signature.asc
Description: Digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCHv2 01/10] drm: omapdrm: panel-dsi-cm: Fix probe for device tree
On Sun 2017-03-05 01:43:00, Sebastian Reichel wrote: > From: Tony Lindgren > > Things are a bit whacked right now for panel-dsi-cm: > > 1. We're missing call to of_get_display_timing() and >videomode_from_timing() > > 2. We need to call dsicm_probe_of() after we initialize the >default values to not overwrite device tree configured >values > > 3. We need to implement minimal get_timings() and check_timings() >for the panel to work > > With these changes we get panel-dsi-cm to probe with device tree > configuraion. Note that the dsicm_check_timings adapted from an Typo: configuraion. Acked-by: Pavel Machek Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCHv2 02/10] drm: omapdrm: panel-dsi-cm: add regulator support
Hi!
> The N950's display requires two regulators.
>
> Signed-off-by: Sebastian Reichel
> ---
> drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c | 55
> +++--
> 1 file changed, 52 insertions(+), 3 deletions(-)
> @@ -590,25 +594,43 @@ static int dsicm_power_on(struct panel_drv_data *ddata)
> .lp_clk_max = 1000,
> };
>
> + if (ddata->vpnl) {
> + r = regulator_enable(ddata->vpnl);
> + if (r) {
> + dev_err(&ddata->pdev->dev,
> + "failed to enable VPNL: %d\n", r);
> + goto err0;
> + }
> + }
Having err0 that just returns .. is quite confusing. Just return here?
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures)
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
signature.asc
Description: Digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] drm/etnaviv: submit support for in-fences
Am Freitag, den 17.03.2017, 14:42 + schrieb Russell King - ARM Linux: > On Fri, Mar 17, 2017 at 03:10:21PM +0100, Lucas Stach wrote: > > Am Donnerstag, den 16.03.2017, 12:05 +0100 schrieb Philipp Zabel: > > > Hi Gustavo, > > > > > > On Mon, 2017-03-13 at 14:37 -0300, Gustavo Padovan wrote: > > > [...] > > > > I was thinking on some function that would iterate over all fences in > > > > the fence_array and check their context. The if we find our own gpu > > > > context in there we fail the submit. > > > > > > Why would we have to fail if somebody feeds us our own fences? Wouldn't > > > it be enough to just wait if there are foreign fences in the array? > > > > Yes, skipping the wait if all fences are from our own context is an > > optimization and it's certainly not an issue if someone feeds us our own > > fences. > > Are you sure about that - what if we have two GPUs, a 2D and 3D GPU, > and we're fed an etnaviv fence for one GPU when submitting to the > other GPU. > > So we do end up being fed our own fences, and we have to respect them > otherwise we lose inter-GPU synchronisation, and that will break > existing userspace. > The etnaviv GPUs, while being on the same DRM device, have distinct fence contexts. So the 3D GPU will consider a fence from the 2D GPU as foreign and properly wait on it. It's only when we get an in fence that has been generated as an out fence by one (or multiple) submits to the same GPU, that we are able to skip the wait and enqueue the command without waiting for the fence to signal. Regards, Lucas ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/amdgpu: remove redundant outer loop and remove commented out code
From: Colin Ian King
The outer loop is redundant and can be removed as it is doing nothing
useful. Also remove some commented out code that is not being used.
Detected by CoverityScan, CID#1402073
Signed-off-by: Colin Ian King
---
drivers/gpu/drm/amd/amdgpu/vce_v2_0.c | 27 +++
1 file changed, 7 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v2_0.c
b/drivers/gpu/drm/amd/amdgpu/vce_v2_0.c
index 9ea99348e493..72dea663f113 100644
--- a/drivers/gpu/drm/amd/amdgpu/vce_v2_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vce_v2_0.c
@@ -273,24 +273,14 @@ static int vce_v2_0_start(struct amdgpu_device *adev)
static int vce_v2_0_stop(struct amdgpu_device *adev)
{
- int i, j;
+ int i;
int status;
if (vce_v2_0_lmi_clean(adev)) {
DRM_INFO("vce is not idle \n");
return 0;
}
-/*
- for (i = 0; i < 10; ++i) {
- for (j = 0; j < 100; ++j) {
- status = RREG32(mmVCE_FW_REG_STATUS);
- if (!(status & 1))
- break;
- mdelay(1);
- }
- break;
- }
-*/
+
if (vce_v2_0_wait_for_idle(adev)) {
DRM_INFO("VCE is busy, Can't set clock gateing");
return 0;
@@ -299,14 +289,11 @@ static int vce_v2_0_stop(struct amdgpu_device *adev)
/* Stall UMC and register bus before resetting VCPU */
WREG32_P(mmVCE_LMI_CTRL2, 1 << 8, ~(1 << 8));
- for (i = 0; i < 10; ++i) {
- for (j = 0; j < 100; ++j) {
- status = RREG32(mmVCE_LMI_STATUS);
- if (status & 0x240)
- break;
- mdelay(1);
- }
- break;
+ for (i = 0; i < 100; ++i) {
+ status = RREG32(mmVCE_LMI_STATUS);
+ if (status & 0x240)
+ break;
+ mdelay(1);
}
WREG32_P(mmVCE_VCPU_CNTL, 0, ~0x80001);
--
2.11.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] drm/etnaviv: submit support for in-fences
Am Freitag, den 17.03.2017, 15:09 +0100 schrieb Philipp Zabel:
> On Fri, 2017-03-17 at 10:55 -0300, Gustavo Padovan wrote:
> > 2017-03-16 Rob Clark :
> >
> > > On Wed, Mar 8, 2017 at 9:37 AM, Gustavo Padovan
> > > wrote:
> > > >> diff --git a/include/uapi/drm/etnaviv_drm.h
> > > >> b/include/uapi/drm/etnaviv_drm.h
> > > >> index 2584c1cca42f6..e9c388a1d8ebe 100644
> > > >> --- a/include/uapi/drm/etnaviv_drm.h
> > > >> +++ b/include/uapi/drm/etnaviv_drm.h
> > > >> @@ -154,6 +154,10 @@ struct drm_etnaviv_gem_submit_bo {
> > > >> * one or more cmdstream buffers. This allows for conditional
> > > >> execution
> > > >> * (context-restore), and IB buffers needed for per tile/bin draw
> > > >> cmds.
> > > >> */
> > > >> +#define ETNA_SUBMIT_NO_IMPLICIT 0x0001
> > > >> +#define ETNA_SUBMIT_FENCE_FD_IN 0x0002
> > > >
> > > > ETNA_SUBMIT_NO_IMPLICIT and ETNA_SUBMIT_FENCE_FD_IN are the same, when
> > > > you send and fence_fd to the kernel you are requesting on explicit sync
> > > > thus I think the ETNA_SUBMIT_NO_IMPLICIT can be dropped and
> > > > ETNA_SUBMIT_FENCE_FD_IN would be the one to look at.
> > >
> > > jfwiw, I kept separate no-implicit and fence-fd-in flags in msm mostly
> > > because I couldn't think of a good backwards-compatible way to add it
> > > later if needed. Currently userspace sets both flags together, and
> > > possibly always will. But keeping separate flags seemed like a good
> > > idea for future-proofing..
> >
> > Fair enough. Let's do the same for etnaviv then.
>
> Alright. Unless Lucas disagrees, I'll keep it as is for consistency.
This flag might make things a bit more "fun" later, as we might need to
merge fences (or even fence arrays of different types) if we are going
to use both implicit and explicit fences to infer scheduling decisions
from.
But to avoid introducing any unnecessary differences between freedreno
and etnaviv, I would suggest to keep the flag around.
Regards,
Lucas
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH 0/3] drm/panel: Pull some code out into common helpers
On Thu, Mar 16, 2017 at 6:08 PM, Sean Paul wrote: > This series pulls out the power-sequencing code from panel-simple into a > panel-common helper library. This allows drivers that cannot leverage > panel-simple to share some code. > > I've converted the 2 sharp mipi drivers, and Chris Zhong's driver on the > list can also be converted. I haven't checked any other drivers, but I > suspect we'll see the same code blocks there too. > > I'm sure there's more we can pull out of the various drivers, but this > seems like a good place to start talking about how to share common panel > code across drivers. > Cc: Noralf Trønnes Cc: Daniel Vetter Adding Noralf to see if this might be useful for tinydrm. Sean > Sean > > Sean Paul (3): > drm/panel: Pull common panel code out into helpers > drm/panel: sharp-lq101r1sx01: Use panel-common helpers > drm/panel: panel-sharp-ls043t1le01: Use panel-common helpers > > drivers/gpu/drm/panel/Kconfig | 22 +++- > drivers/gpu/drm/panel/Makefile | 1 + > drivers/gpu/drm/panel/panel-common.c| 149 > > drivers/gpu/drm/panel/panel-common.h| 44 +++ > drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c | 79 - > drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c | 70 +++ > drivers/gpu/drm/panel/panel-simple.c| 112 +++--- > 7 files changed, 269 insertions(+), 208 deletions(-) > create mode 100644 drivers/gpu/drm/panel/panel-common.c > create mode 100644 drivers/gpu/drm/panel/panel-common.h > > -- > 2.12.0.367.g23dc2f6d3c-goog > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] drm/etnaviv: submit support for in-fences
Am Donnerstag, den 16.03.2017, 12:05 +0100 schrieb Philipp Zabel:
> Hi Gustavo,
>
> On Mon, 2017-03-13 at 14:37 -0300, Gustavo Padovan wrote:
> [...]
> > I was thinking on some function that would iterate over all fences in
> > the fence_array and check their context. The if we find our own gpu
> > context in there we fail the submit.
>
> Why would we have to fail if somebody feeds us our own fences? Wouldn't
> it be enough to just wait if there are foreign fences in the array?
Yes, skipping the wait if all fences are from our own context is an
optimization and it's certainly not an issue if someone feeds us our own
fences.
>
> How about something like this:
>
> --8<--
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> index 364fe50d020de..0b0bdaf4406d4 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> @@ -296,6 +296,22 @@ static void submit_cleanup(struct etnaviv_gem_submit
> *submit)
> kfree(submit);
> }
>
> +static bool dma_fence_all_in_context(struct dma_fence *fence, u64 context)
dma_fence_match_context?
> +{
> + if (dma_fence_is_array(fence)) {
> + struct dma_fence_array *array = to_dma_fence_array(fence);
> + int i;
> +
> + for (i = 0; i < array->num_fences; i++) {
> + if (array->fences[i]->context != context)
> + return false;
> + }
> + return true;
> + }
> +
> + return fence->context == context;
> +}
> +
This looks good to me. Please add this to a common location in the next
submission.
> int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data,
> struct drm_file *file)
> {
> @@ -413,12 +429,11 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev,
> void *data,
> goto err_submit_objects;
> }
>
> - /* TODO if we get an array-fence due to userspace merging
> - * multiple fences, we need a way to determine if all the
> - * backing fences are from our own context..
> + /*
> + * Wait if the fence is from a foreign context, or if the fence
> + * array contains any fence from a foreign context.
>*/
> -
> - if (in_fence->context != gpu->fence_context) {
> + if (!dma_fence_all_in_context(in_fence, gpu->fence_context)) {
> ret = dma_fence_wait(in_fence, true);
> if (ret)
> goto err_submit_objects;
> -->8--
>
> [...]
> > > > > @@ -154,6 +154,10 @@ struct drm_etnaviv_gem_submit_bo {
> > > > > * one or more cmdstream buffers. This allows for conditional
> > > > > execution
> > > > > * (context-restore), and IB buffers needed for per tile/bin draw
> > > > > cmds.
> > > > > */
> > > > > +#define ETNA_SUBMIT_NO_IMPLICIT 0x0001
> > > > > +#define ETNA_SUBMIT_FENCE_FD_IN 0x0002
> > > >
> > > > ETNA_SUBMIT_NO_IMPLICIT and ETNA_SUBMIT_FENCE_FD_IN are the same, when
> > > > you send and fence_fd to the kernel you are requesting on explicit sync
> > > > thus I think the ETNA_SUBMIT_NO_IMPLICIT can be dropped and
> > > > ETNA_SUBMIT_FENCE_FD_IN would be the one to look at.
> > >
> > > I just followed Rob's lead. I'm not sure if there may be a reason to
> > > submit an in fence still keep implicit fencing enabled at the same time,
> > > or to allow a submit without any fencing whatsoever. Maybe for testing
> > > purposes?
> > > I'm happy to drop the NO_IMPLICIT flag if there is no need for it.
> >
> > Right. My understanding is that the flags would mean the same thing, but
> > I'm no expert on the GPU side of things. Maybe Rob can provide us some
> > insight on why he added NO_IMPLICIT.
>
> Yes, that would be welcome.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: DRM Atomic property for color-space conversion
On Fri, Mar 17, 2017 at 10:33:15AM +, Brian Starkey wrote: > Hi Ville, > > On Thu, Mar 16, 2017 at 07:36:56PM +0200, Ville Syrjälä wrote: > >On Thu, Mar 16, 2017 at 07:05:12PM +0200, Sharma, Shashank wrote: > >> On 3/16/2017 5:55 PM, Brian Starkey wrote: > >> > On Thu, Mar 16, 2017 at 05:14:07PM +0200, Sharma Shashank wrote: > >> >> On 3/16/2017 4:37 PM, Local user for Liviu Dudau wrote: > >> >>> On Thu, Mar 16, 2017 at 04:30:59PM +0200, Ville Syrjälä wrote: > >> On Thu, Mar 16, 2017 at 04:20:29PM +0200, Sharma, Shashank wrote: > >> > On 3/16/2017 4:07 PM, Ville Syrjälä wrote: > > [snip] > > >> >> > >> >> So what we might need is something like: > >> >> enum YCBCR_TO_RGB_CSC > >> >> * YCbCr BT.601 limited to RGB BT.709 full > >> >> * YCbCr BT.709 limited to RGB BT.709 full >> >> likely default value IMO> > >> >> * YCbCr BT.601 limited to RGB BT.2020 full > >> >> * YCbCr BT.709 limited to RGB BT.2020 full > >> >> * YCbCr BT.2020 limited to RGB BT.2020 full > >> >> > >> >> And thanks to BT.2020 we'll need a RGB->RGB CSC property as well. > >> >> Eg: > >> >> enum RGB_TO_RGB_CSC > >> >> * bypass (or separate 709->709, 2020->2020?) >> >> default> > >> >> * RGB BT.709 full to RGB BT.2020 full > >> > > >> > I like this approach, from a point of view of being explicit and > >> > discoverable by userspace. It also happens to map quite nicely to our > >> > hardware... we have a matrix before degamma, so we could do a > >> > CSC + Gamut conversion there in one go, which is apparently not 100% > >> > mathematically correct, but in general is good enough. > >> > > >> > ... however having talked this over a bit with someone who understands > >> > the detail a lot better than me, it sounds like the "correct" thing to > >> > do as per the spec is: > >> > > >> > CSC -> DEGAMMA -> GAMUT > >> > > >> > e.g. > >> > > >> > YCbCr bt.601 limited to RGB bt.601 full -> degamma -> > >> > RGB bt.601 full to RGB bt.709 full > >> > > >> > So that sounds like what we need to support in the API, and also > >> > sounds more like the "separate properties" approach. > >> I agree. > >> > > >> >> > >> >> Alternatives would involve two properties to define the input and > >> >> output > >> >> from the CSC separately, but then you lose the capability to see > >> >> which > >> >> combinations are actually supoorted. > >> > I was thinking about this too, or would it make more sense to > >> > create two > >> > properties: > >> > - one for gamut mapping (cases like RGB709->RGB2020) > >> > - other one for Color space conversion (cases lile YUV 709 -> RGB > >> > 709) > >> > > >> > Gamut mapping can represent any of the fix function mapping, > >> > wereas CSC > >> > can bring up any programmable matrix > >> > > >> > Internally these properties can use the same HW unit or even same > >> > function. > >> > Does it sound any good ? > >> > > >> > It seems to me that actually the two approaches can be combined into > >> > the same thing: > >> > * We definitely need a YCbCr-to-RGB conversion before degamma > >> >(for converting YUV data to RGB, in some flavour) > >> > * We definitely need an RGB-to-RGB conversion after gamma to handle > >> >709 layers blended with Rec.2020. > >> > The exact conversion each of those properties represents (CSC + gamut, > >> > CSC only, gamut only) can be implicit in the enum name. > >> > > >> > For hardware which has a fixed-function CSC before DEGAMMA with a > >> > matrix after DEGAMMA, I'd expect to see something like below. None of > >> > the YCBCR_TO_RGB_CSC values include a gamut conversion, because that > >> > is instead exposed with the RGB_TO_RGB_CSC property (which represents > >> > the hardware matrix) > >> > > >> > YCBCR_TO_RGB_CSC (before DEGAMMA): > >> > YCbCr BT.601 limited to RGB BT.601 full > >> > YCbCr BT.709 limited to RGB BT.709 full > >> > YCbCr BT.2020 limited to RGB BT.2020 full > >> > > >> > RGB_TO_RGB_CSC (after DEGAMMA): > >> > RGB BT.601 full to RGB BT.709 full > >> > RGB BT.709 full to RGB BT.2020 full > >> > > >> > > >> > On the other hand, on hardware which does a CSC + Gamut conversion in > >> > one go, before DEGAMMA (like ours), you might have: > >> > > >> > YCBCR_TO_RGB_CSC (before DEGAMMA): > >> > YCbCr BT.601 limited to RGB BT.601 full > >> > YCbCr BT.601 limited to RGB BT.709 full > >> > YCbCr BT.709 limited to RGB BT.709 full > >> > YCbCr BT.2020 limited to RGB BT.2020 full > >> > > >> > RGB_TO_RGB_CSC (after DEGAMMA): > >> > Not supported > >> > > >> > Userspace can parse the two properties to figure out its options to > >> > get from desired input -> desired output. It is perhaps a little > >> > verbose, but it's descriptive and flexible. > >> > > >> Seems to be a good idea, Ville ? > > > >Looks pretty sane to me. > > > >Though how we'll do the degamma/gamma is
Re: [PATCH 1/3] drm/etnaviv: submit support for in-fences
On Fri, 2017-03-17 at 10:55 -0300, Gustavo Padovan wrote:
> 2017-03-16 Rob Clark :
>
> > On Wed, Mar 8, 2017 at 9:37 AM, Gustavo Padovan wrote:
> > >> diff --git a/include/uapi/drm/etnaviv_drm.h
> > >> b/include/uapi/drm/etnaviv_drm.h
> > >> index 2584c1cca42f6..e9c388a1d8ebe 100644
> > >> --- a/include/uapi/drm/etnaviv_drm.h
> > >> +++ b/include/uapi/drm/etnaviv_drm.h
> > >> @@ -154,6 +154,10 @@ struct drm_etnaviv_gem_submit_bo {
> > >> * one or more cmdstream buffers. This allows for conditional execution
> > >> * (context-restore), and IB buffers needed for per tile/bin draw cmds.
> > >> */
> > >> +#define ETNA_SUBMIT_NO_IMPLICIT 0x0001
> > >> +#define ETNA_SUBMIT_FENCE_FD_IN 0x0002
> > >
> > > ETNA_SUBMIT_NO_IMPLICIT and ETNA_SUBMIT_FENCE_FD_IN are the same, when
> > > you send and fence_fd to the kernel you are requesting on explicit sync
> > > thus I think the ETNA_SUBMIT_NO_IMPLICIT can be dropped and
> > > ETNA_SUBMIT_FENCE_FD_IN would be the one to look at.
> >
> > jfwiw, I kept separate no-implicit and fence-fd-in flags in msm mostly
> > because I couldn't think of a good backwards-compatible way to add it
> > later if needed. Currently userspace sets both flags together, and
> > possibly always will. But keeping separate flags seemed like a good
> > idea for future-proofing..
>
> Fair enough. Let's do the same for etnaviv then.
Alright. Unless Lucas disagrees, I'll keep it as is for consistency.
regards
Philipp
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] drm/etnaviv: submit support for in-fences
On Fri, 2017-03-17 at 11:00 -0300, Gustavo Padovan wrote: > 2017-03-16 Philipp Zabel : > > > Hi Gustavo, > > > > On Mon, 2017-03-13 at 14:37 -0300, Gustavo Padovan wrote: > > [...] > > > I was thinking on some function that would iterate over all fences in > > > the fence_array and check their context. The if we find our own gpu > > > context in there we fail the submit. > > > > Why would we have to fail if somebody feeds us our own fences? Wouldn't > > it be enough to just wait if there are foreign fences in the array? > > You don't need to fail necessarily. In my mind I had the use case that > maybe some driver could deadlock waiting for his own fence. What you > do with the information that the array has it a fence with the driver's > context is entirely up to the driver to decide. [...] > If we don't mind having fences with our own context, why should we check > them? My understanding was that for foreign fences we have to wait, for example before resolving into a framebuffer that is still being scanned out. But if we are fed fences from our own context, we can just ignore them because etnaviv just has a single command queue, so anything waiting for a fence from our own context will be queued after that fence is signaled anyway. regards Philipp ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] drm/etnaviv: submit support for in-fences
2017-03-16 Philipp Zabel :
> Hi Gustavo,
>
> On Mon, 2017-03-13 at 14:37 -0300, Gustavo Padovan wrote:
> [...]
> > I was thinking on some function that would iterate over all fences in
> > the fence_array and check their context. The if we find our own gpu
> > context in there we fail the submit.
>
> Why would we have to fail if somebody feeds us our own fences? Wouldn't
> it be enough to just wait if there are foreign fences in the array?
You don't need to fail necessarily. In my mind I had the use case that
maybe some driver could deadlock waiting for his own fence. What you
do with the information that the array has it a fence with the driver's
context is entirely up to the driver to decide.
>
> How about something like this:
>
> --8<--
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> index 364fe50d020de..0b0bdaf4406d4 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> @@ -296,6 +296,22 @@ static void submit_cleanup(struct etnaviv_gem_submit
> *submit)
> kfree(submit);
> }
>
> +static bool dma_fence_all_in_context(struct dma_fence *fence, u64 context)
> +{
> + if (dma_fence_is_array(fence)) {
> + struct dma_fence_array *array = to_dma_fence_array(fence);
> + int i;
> +
> + for (i = 0; i < array->num_fences; i++) {
> + if (array->fences[i]->context != context)
> + return false;
> + }
> + return true;
> + }
> +
> + return fence->context == context;
> +}
If we don't mind having fences with our own context, why should we check
them?
Gustavo
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] drm/etnaviv: submit support for in-fences
2017-03-16 Rob Clark :
> On Wed, Mar 8, 2017 at 9:37 AM, Gustavo Padovan wrote:
> >> diff --git a/include/uapi/drm/etnaviv_drm.h
> >> b/include/uapi/drm/etnaviv_drm.h
> >> index 2584c1cca42f6..e9c388a1d8ebe 100644
> >> --- a/include/uapi/drm/etnaviv_drm.h
> >> +++ b/include/uapi/drm/etnaviv_drm.h
> >> @@ -154,6 +154,10 @@ struct drm_etnaviv_gem_submit_bo {
> >> * one or more cmdstream buffers. This allows for conditional execution
> >> * (context-restore), and IB buffers needed for per tile/bin draw cmds.
> >> */
> >> +#define ETNA_SUBMIT_NO_IMPLICIT 0x0001
> >> +#define ETNA_SUBMIT_FENCE_FD_IN 0x0002
> >
> > ETNA_SUBMIT_NO_IMPLICIT and ETNA_SUBMIT_FENCE_FD_IN are the same, when
> > you send and fence_fd to the kernel you are requesting on explicit sync
> > thus I think the ETNA_SUBMIT_NO_IMPLICIT can be dropped and
> > ETNA_SUBMIT_FENCE_FD_IN would be the one to look at.
>
> jfwiw, I kept separate no-implicit and fence-fd-in flags in msm mostly
> because I couldn't think of a good backwards-compatible way to add it
> later if needed. Currently userspace sets both flags together, and
> possibly always will. But keeping separate flags seemed like a good
> idea for future-proofing..
Fair enough. Let's do the same for etnaviv then.
Gustavo
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 194649] Graphical garbage r9 380
https://bugzilla.kernel.org/show_bug.cgi?id=194649 --- Comment #10 from Roberth Sjonøy (roberth.sjo...@gmail.com) --- The issue became even more prevelant when upgrading from kernel 4.10.2 to 4.10.3. -- 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 100242] radeon buffer allocation failure during startup of Factorio
https://bugs.freedesktop.org/show_bug.cgi?id=100242 --- Comment #4 from Christian König --- The maximum size for allocations in VRAM on radeon are usually 256MB minus pinned system resources like GART. The game tries to allocate something which is 256MB in size, so that won't fit. Either the driver tries to allocate a temporary buffer which is way to large or the game tries to allocate a texture which is way to large. -- 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 100242] radeon buffer allocation failure during startup of Factorio
https://bugs.freedesktop.org/show_bug.cgi?id=100242 --- Comment #3 from tnmailingli...@gmail.com --- I retested the game with mesa 12.0.6 and xserver-xorg-video-radeon 1:7.7.1 (the current versions for default Lubuntu 16.10) and noticed that while the game doesn't crash, it still produces buffer allocation failures during startup: 2.969 Initial atlas bitmap size is 8192 radeon: Failed to allocate a buffer: radeon:size : 268435456 bytes radeon:alignment : 16384 bytes radeon:domains : 4 radeon:flags : 20 radeon: Failed to allocate a buffer: radeon:size : 268435456 bytes radeon:alignment : 16384 bytes radeon:domains : 4 radeon:flags : 20 radeon: Failed to allocate a buffer: radeon:size : 268435456 bytes radeon:alignment : 16384 bytes radeon:domains : 4 radeon:flags : 20 radeon: Failed to allocate a buffer: radeon:size : 268435456 bytes radeon:alignment : 16384 bytes radeon:domains : 4 radeon:flags : 20 3.029 Couldn't create atlas bitmap (size 8192x8182), will try again 3.094 Created atlas bitmap 4096x4096 -- 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: Mark up accesses of vblank->enabled outside of its spinlock
On Fri, Mar 17, 2017 at 10:19:51AM +, Chris Wilson wrote:
> On Fri, Mar 17, 2017 at 11:47:51AM +0200, Ville Syrjälä wrote:
> > On Thu, Mar 16, 2017 at 11:47:48PM +, Chris Wilson wrote:
> > > @@ -360,7 +358,7 @@ static void vblank_disable_fn(unsigned long arg)
> > > unsigned long irqflags;
> > >
> > > spin_lock_irqsave(&dev->vbl_lock, irqflags);
> > > - if (atomic_read(&vblank->refcount) == 0 && vblank->enabled) {
> > > + if (atomic_read(&vblank->refcount) == 0 && READ_ONCE(vblank->enabled)) {
> >
> > Hmm. Aren't most of these accesses inside the lock? Looks like you're
> > marking everything READ/WRITE_ONCE()?
>
> There's like 3 different locks here. Afaict, the correct one for
> serialising vblank->enabled was dev->vblank_time_lock. Every access
> outside of that lock, I marked up as READ_ONCE.
>
> Oh, you are using vbl_lock as the barrier? That's not as clear for
> disable:
>
> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> index 53a526c7b24d..f447ed07ef95 100644
> --- a/drivers/gpu/drm/drm_irq.c
> +++ b/drivers/gpu/drm/drm_irq.c
> @@ -325,6 +325,8 @@ static void vblank_disable_and_save(struct drm_device
> *dev, unsigned int pipe)
> struct drm_vblank_crtc *vblank = &dev->vblank[pipe];
> unsigned long irqflags;
>
> + assert_spin_locked(&dev->vbl_lock);
> +
> /* Prevent vblank irq processing while disabling vblank irqs,
> * so no updates of timestamps or count can happen after we've
> * disabled. Needed to prevent races in case of delayed irq's.
>
>
> > > @@ -1714,6 +1717,9 @@ bool drm_handle_vblank(struct drm_device *dev,
> > > unsigned int pipe)
> > > if (WARN_ON(pipe >= dev->num_crtcs))
> > > return false;
> > >
> > > + if (!READ_ONCE(vblank->enabled))
> > > + return false;
> >
> > This to me looks like it could theoretically cause us to
> > miss an interrupt.
> >
> > 1. enable_irq()
> > 2. drm_update_vblank_count()
> > 3. irq fires
> > 4. drm_handle_vblank() doesn't do anything
> > 5. enabled=true
>
> Sure. There's a danger you miss the irq anyway, and so the last action
> after enabling the interrupt should be to process any completed events -
> that's implicitly handled by enabling the interrupt in advance of adding
> the first event. In the scenario above, there should be nothing to do.
That's a slightly different scenario since you're only thinking about
actually enabling the interrupt vs. calling drm_update_vblank_count().
That is fine as is. But with the extra 'enabled' check in the interrupt
handler you're effectively reversing that order to enable the interrupt
after drm_update_vblank_count(). Hence we can lose an interrupt now.
Of course we would eventually get another interrupt, and thanks to the
hw frame counter and whatnot we wouldn't actually lose our place entirely,
but we would see the counter jump by two frames when we actually handle
the interrupt.
--
Ville Syrjälä
Intel OTC
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/fb-helper: Only reject FB changes if FB_MISC_USER_EVENT is set
On Fri, Mar 17, 2017 at 07:19:27PM +0900, Michel Dänzer wrote: > On 17/03/17 07:01 PM, Ville Syrjälä wrote: > > On Fri, Mar 17, 2017 at 06:01:41PM +0900, Michel Dänzer wrote: > >> On 16/03/17 07:09 PM, Ville Syrjälä wrote: > >>> On Thu, Mar 16, 2017 at 06:55:53PM +0900, Michel Dänzer wrote: > From: Michel Dänzer > > Otherwise this can also prevent modesets e.g. for switching VTs. > > FB_MISC_USER_EVENT is set when the request originates from userspace, > which is what we're interested in here according to the DRM_DEBUG > output. > >>> > >>> So why is the kernel allowed to violate this? > >>> > >>> The checks look somewhat bogus to me anyway. The 'virtual size == fb size' > >>> check makes some kind of sense. Although I don't see why the virtual > >>> resolution couldn't be smaller than the fb size. But requiring that the > >>> visible resolutionn matches the fb size as well definitely seems wrong. > >> > >> Agreed on all counts. So, I think what's needed is almost a revert of > >> 865afb11949e, except for the bits_per_pixel comparison, since we really > >> can't change that. See diff below. > > > > So one semi-crazy idea I had was: > > 12:18 < vsyrjala> daniels: hmm. given that the fb_helper doesn't > > implement a modeset in set_par, i guess what we really > > should do is look at the currently active crtcs and see if the mode on > > one of them more or less matches what the user is asking for > > I don't get the idea. What's the point of comparing the var->* values to > whatever is currently active in the hardware? Not currently active in the hardware, but currently active as far as fb_helper is concerned. I guess I should say "matches fb_helper's current crtc configuration" or something. > The purpose of the code is > to check that the requested parameters fit into fb_helper's framebuffer. > > > > I tried to trip this current check by booting with a big screen and > > later switching to a small screen. For some reason that worked out > > just fine, > > I'd need more details about what exactly you tried. > > > so I'm not even sure what kind of values we stuff into xres & co. > > It should be the values shown / attempted to set by fbset. I meant what does the kernel put there for fbcon etc. I was expecting that it xres/yres would be the visible resolution of the smallest screen, and the virtual resolution would be either the same or the size of the fb perhaps. But that can't be the case or else my experiment would have produced a failure. So I would have to dump that stuff out to see what really ends up there. -- Ville Syrjälä Intel OTC ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: add check for plane functions
On Fri, Mar 17, 2017 at 03:46:34PM +0530, Shirish S wrote:
> On Fri, Mar 17, 2017 at 3:26 PM, Ville Syrjälä
> wrote:
> > On Fri, Mar 17, 2017 at 01:25:08PM +0530, Shirish S wrote:
> >> update_plane() and disable_plane() functions
> >> assoiciated with setting plane are called
> >> without any check, causing kernel panic.
> >
> > Why are you registering a plane without the funcs?
> >
> Basically, enabling planes and making them fully functional is
> generally a 2 -step process,
> so i suggest for new drivers wanting to implement/re-design planes,
> would like to tap
> the flow at enabling(listing caps) and later at ensuring it works.
I don't think there's much point in exposing something that
doesn't work. And even if you do, you could always just use
stub functions.
> I noticed that there is a underlying assumption only for
> plane->(funcs) are implemented, whereas for
> other function for crtc/connector/encoder function calls there is a
> sanity check(or WARN_ON) through out the framework.
>
> I believe this check wont cause any performance/functional impact.
> Please let me know if am missing anything.
> And further more help developers to focus on enabling planes via
> various tests without causing reboots/system hangs.
I don't particularly like adding more unconditional runtime checks
that just to protect developers from themselves. If you really
think there's value in these, then at least add the checks into
the plane init codepath so that it's a one time cost.
The same approach could be used for all the other non-optional
hooks. Otherwise the same WARN_ON()s would have to be sprinkled
all over the place, and there's always the risk of missing a few
codepaths that call a specific hook.
> >>
> >> This patch adds the required check to avoid it.
> >>
> >> Change-Id: I0d6792608b33e674c217388aa57c4b7d680d9bc7
> >> Signed-off-by: Shirish S
> >> ---
> >> drivers/gpu/drm/drm_plane.c | 6 ++
> >> 1 file changed, 6 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> >> index 249c0ae..f675f8b 100644
> >> --- a/drivers/gpu/drm/drm_plane.c
> >> +++ b/drivers/gpu/drm/drm_plane.c
> >> @@ -456,6 +456,12 @@ static int __setplane_internal(struct drm_plane
> >> *plane,
> >> {
> >> int ret = 0;
> >>
> >> + if (plane->funcs->disable_plane == NULL ||
> >> + plane->funcs->update_plane == NULL) {
> >> + DRM_ERROR("plane funcs not implemented\n");
> >> + ret = -EPERM;
> >> + goto out;
> >> + }
> >> /* No fb means shut it down */
> >> if (!fb) {
> >> plane->old_fb = plane->fb;
> >> --
> >> 2.7.4
> >>
> >> ___
> >> dri-devel mailing list
> >> dri-devel@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
> > --
> > Ville Syrjälä
> > Intel OTC
--
Ville Syrjälä
Intel OTC
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: DRM Atomic property for color-space conversion
Hi Ville, On Thu, Mar 16, 2017 at 07:36:56PM +0200, Ville Syrjälä wrote: On Thu, Mar 16, 2017 at 07:05:12PM +0200, Sharma, Shashank wrote: On 3/16/2017 5:55 PM, Brian Starkey wrote: > On Thu, Mar 16, 2017 at 05:14:07PM +0200, Sharma Shashank wrote: >> On 3/16/2017 4:37 PM, Local user for Liviu Dudau wrote: >>> On Thu, Mar 16, 2017 at 04:30:59PM +0200, Ville Syrjälä wrote: On Thu, Mar 16, 2017 at 04:20:29PM +0200, Sharma, Shashank wrote: > On 3/16/2017 4:07 PM, Ville Syrjälä wrote: [snip] >> >> So what we might need is something like: >> enum YCBCR_TO_RGB_CSC >> * YCbCr BT.601 limited to RGB BT.709 full >> * YCbCr BT.709 limited to RGB BT.709 full > likely default value IMO> >> * YCbCr BT.601 limited to RGB BT.2020 full >> * YCbCr BT.709 limited to RGB BT.2020 full >> * YCbCr BT.2020 limited to RGB BT.2020 full >> >> And thanks to BT.2020 we'll need a RGB->RGB CSC property as well. >> Eg: >> enum RGB_TO_RGB_CSC >> * bypass (or separate 709->709, 2020->2020?) > default> >> * RGB BT.709 full to RGB BT.2020 full > > I like this approach, from a point of view of being explicit and > discoverable by userspace. It also happens to map quite nicely to our > hardware... we have a matrix before degamma, so we could do a > CSC + Gamut conversion there in one go, which is apparently not 100% > mathematically correct, but in general is good enough. > > ... however having talked this over a bit with someone who understands > the detail a lot better than me, it sounds like the "correct" thing to > do as per the spec is: > > CSC -> DEGAMMA -> GAMUT > > e.g. > > YCbCr bt.601 limited to RGB bt.601 full -> degamma -> > RGB bt.601 full to RGB bt.709 full > > So that sounds like what we need to support in the API, and also > sounds more like the "separate properties" approach. I agree. > >> >> Alternatives would involve two properties to define the input and >> output >> from the CSC separately, but then you lose the capability to see >> which >> combinations are actually supoorted. > I was thinking about this too, or would it make more sense to > create two > properties: > - one for gamut mapping (cases like RGB709->RGB2020) > - other one for Color space conversion (cases lile YUV 709 -> RGB > 709) > > Gamut mapping can represent any of the fix function mapping, > wereas CSC > can bring up any programmable matrix > > Internally these properties can use the same HW unit or even same > function. > Does it sound any good ? > > It seems to me that actually the two approaches can be combined into > the same thing: > * We definitely need a YCbCr-to-RGB conversion before degamma >(for converting YUV data to RGB, in some flavour) > * We definitely need an RGB-to-RGB conversion after gamma to handle >709 layers blended with Rec.2020. > The exact conversion each of those properties represents (CSC + gamut, > CSC only, gamut only) can be implicit in the enum name. > > For hardware which has a fixed-function CSC before DEGAMMA with a > matrix after DEGAMMA, I'd expect to see something like below. None of > the YCBCR_TO_RGB_CSC values include a gamut conversion, because that > is instead exposed with the RGB_TO_RGB_CSC property (which represents > the hardware matrix) > > YCBCR_TO_RGB_CSC (before DEGAMMA): > YCbCr BT.601 limited to RGB BT.601 full > YCbCr BT.709 limited to RGB BT.709 full > YCbCr BT.2020 limited to RGB BT.2020 full > > RGB_TO_RGB_CSC (after DEGAMMA): > RGB BT.601 full to RGB BT.709 full > RGB BT.709 full to RGB BT.2020 full > > > On the other hand, on hardware which does a CSC + Gamut conversion in > one go, before DEGAMMA (like ours), you might have: > > YCBCR_TO_RGB_CSC (before DEGAMMA): > YCbCr BT.601 limited to RGB BT.601 full > YCbCr BT.601 limited to RGB BT.709 full > YCbCr BT.709 limited to RGB BT.709 full > YCbCr BT.2020 limited to RGB BT.2020 full > > RGB_TO_RGB_CSC (after DEGAMMA): > Not supported > > Userspace can parse the two properties to figure out its options to > get from desired input -> desired output. It is perhaps a little > verbose, but it's descriptive and flexible. > Seems to be a good idea, Ville ? Looks pretty sane to me. Though how we'll do the degamma/gamma is rather unclear still. I think we might be looking at two variants of hardware: - A fully programmable one with separate stages: csc -> degamma -> gamut -> gamma - A totally fixed one with just a few different variants of the pipeline baked into the hardware If we want to expose the gamma/degamma to the user, how exactly are we going to do that with the latter form or hardware. I guess we could specify that if the degamma property is not exposed, there will be an implicit degamma stage between the two csc and gamut. And if it is exposed the output from the first csc is non-linear and thus needs the degamma pr
Re: [PATCH 1/2] drm: Mark up accesses of vblank->enabled outside of its spinlock
On Fri, Mar 17, 2017 at 11:47:51AM +0200, Ville Syrjälä wrote:
> On Thu, Mar 16, 2017 at 11:47:48PM +, Chris Wilson wrote:
> > @@ -360,7 +358,7 @@ static void vblank_disable_fn(unsigned long arg)
> > unsigned long irqflags;
> >
> > spin_lock_irqsave(&dev->vbl_lock, irqflags);
> > - if (atomic_read(&vblank->refcount) == 0 && vblank->enabled) {
> > + if (atomic_read(&vblank->refcount) == 0 && READ_ONCE(vblank->enabled)) {
>
> Hmm. Aren't most of these accesses inside the lock? Looks like you're
> marking everything READ/WRITE_ONCE()?
There's like 3 different locks here. Afaict, the correct one for
serialising vblank->enabled was dev->vblank_time_lock. Every access
outside of that lock, I marked up as READ_ONCE.
Oh, you are using vbl_lock as the barrier? That's not as clear for
disable:
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index 53a526c7b24d..f447ed07ef95 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -325,6 +325,8 @@ static void vblank_disable_and_save(struct drm_device *dev,
unsigned int pipe)
struct drm_vblank_crtc *vblank = &dev->vblank[pipe];
unsigned long irqflags;
+ assert_spin_locked(&dev->vbl_lock);
+
/* Prevent vblank irq processing while disabling vblank irqs,
* so no updates of timestamps or count can happen after we've
* disabled. Needed to prevent races in case of delayed irq's.
> > @@ -1714,6 +1717,9 @@ bool drm_handle_vblank(struct drm_device *dev,
> > unsigned int pipe)
> > if (WARN_ON(pipe >= dev->num_crtcs))
> > return false;
> >
> > + if (!READ_ONCE(vblank->enabled))
> > + return false;
>
> This to me looks like it could theoretically cause us to
> miss an interrupt.
>
> 1. enable_irq()
> 2. drm_update_vblank_count()
> 3. irq fires
> 4. drm_handle_vblank() doesn't do anything
> 5. enabled=true
Sure. There's a danger you miss the irq anyway, and so the last action
after enabling the interrupt should be to process any completed events -
that's implicitly handled by enabling the interrupt in advance of adding
the first event. In the scenario above, there should be nothing to do.
-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
Re: [PATCH] drm/fb-helper: Only reject FB changes if FB_MISC_USER_EVENT is set
On 17/03/17 07:01 PM, Ville Syrjälä wrote: > On Fri, Mar 17, 2017 at 06:01:41PM +0900, Michel Dänzer wrote: >> On 16/03/17 07:09 PM, Ville Syrjälä wrote: >>> On Thu, Mar 16, 2017 at 06:55:53PM +0900, Michel Dänzer wrote: From: Michel Dänzer Otherwise this can also prevent modesets e.g. for switching VTs. FB_MISC_USER_EVENT is set when the request originates from userspace, which is what we're interested in here according to the DRM_DEBUG output. >>> >>> So why is the kernel allowed to violate this? >>> >>> The checks look somewhat bogus to me anyway. The 'virtual size == fb size' >>> check makes some kind of sense. Although I don't see why the virtual >>> resolution couldn't be smaller than the fb size. But requiring that the >>> visible resolutionn matches the fb size as well definitely seems wrong. >> >> Agreed on all counts. So, I think what's needed is almost a revert of >> 865afb11949e, except for the bits_per_pixel comparison, since we really >> can't change that. See diff below. > > So one semi-crazy idea I had was: > 12:18 < vsyrjala> daniels: hmm. given that the fb_helper doesn't > implement a modeset in set_par, i guess what we really > should do is look at the currently active crtcs and see if the mode on > one of them more or less matches what the user is asking for I don't get the idea. What's the point of comparing the var->* values to whatever is currently active in the hardware? The purpose of the code is to check that the requested parameters fit into fb_helper's framebuffer. > I tried to trip this current check by booting with a big screen and > later switching to a small screen. For some reason that worked out > just fine, I'd need more details about what exactly you tried. > so I'm not even sure what kind of values we stuff into xres & co. It should be the values shown / attempted to set by fbset. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: DRM Atomic property for color-space conversion
On Thu, Mar 16, 2017 at 07:36:56PM +0200, Ville Syrjälä wrote: > On Thu, Mar 16, 2017 at 07:05:12PM +0200, Sharma, Shashank wrote: > > Regards > > > > Shashank > > > > > > On 3/16/2017 5:55 PM, Brian Starkey wrote: > > > Hi, > > > > > > On Thu, Mar 16, 2017 at 05:14:07PM +0200, Sharma Shashank wrote: > > >> Regards > > >> > > >> Shashank > > >> > > >> > > >> On 3/16/2017 4:37 PM, Local user for Liviu Dudau wrote: > > >>> On Thu, Mar 16, 2017 at 04:30:59PM +0200, Ville Syrjälä wrote: > > On Thu, Mar 16, 2017 at 04:20:29PM +0200, Sharma, Shashank wrote: > > > Regards > > > > > > Shashank > > > > > > > > > On 3/16/2017 4:07 PM, Ville Syrjälä wrote: > > >> On Tue, Jan 31, 2017 at 03:55:41PM +, Brian Starkey wrote: > > >>> On Tue, Jan 31, 2017 at 05:15:46PM +0200, Ville Syrjälä wrote: > > On Tue, Jan 31, 2017 at 12:33:29PM +, Brian Starkey wrote: > > > Hi, > > > > > > On Mon, Jan 30, 2017 at 03:35:13PM +0200, Ville Syrjälä wrote: > > >> On Fri, Jan 27, 2017 at 05:23:24PM +, Brian Starkey wrote: > > >>> Hi, > > >>> > > >>> We're looking to enable the per-plane color management > > >>> hardware in > > >>> Mali-DP with atomic properties, which has sparked some > > >>> conversation > > >>> around how to handle YCbCr formats. > > >>> > > >>> As it stands today, it's assumed that a driver will > > >>> implicitly "do the > > >>> right thing" to display a YCbCr buffer. > > >>> > > >>> YCbCr data often uses different gamma curves and signal > > >>> ranges (e.g. > > >>> BT.609, BT.701, BT.2020, studio range, full-range), so its > > >>> desirable > > >>> to be able to explicitly control the YCbCr to RGB conversion > > >>> process > > >>> from userspace. > > >>> > > >>> We're proposing adding a "CSC" (color-space conversion) > > >>> property to > > >>> control this - primarily per-plane for framebuffer->pipeline > > >>> CSC, but > > >>> perhaps one per CRTC too for devices which have an RGB > > >>> pipeline and > > >>> want to output in YUV to the display: > > >>> > > >>> Name: "CSC" > > >>> Type: ENUM | ATOMIC; > > >>> Enum values (representative): > > >>> "default": > > >>> Same behaviour as now. "Some kind" of YCbCr->RGB conversion > > >>> for YCbCr buffers, bypass for RGB buffers > > >>> "disable": > > >>> Explicitly disable all colorspace conversion (i.e. use an > > >>> identity matrix). > > >>> "YCbCr to RGB: BT.709": > > >>> Only valid for YCbCr formats. CSC in accordance with BT.709 > > >>> using [16..235] for (8-bit) luma values, and [16..240] for > > >>> 8-bit chroma values. For 10-bit formats, the range > > >>> limits are > > >>> multiplied by 4. > > >>> "YCbCr to RGB: BT.709 full-swing": > > >>> Only valid for YCbCr formats. CSC in accordance with > > >>> BT.709, > > >>> but using the full range of each channel. > > >>> "YCbCr to RGB: Use CTM":* > > >>> Only valid for YCbCr formats. Use the matrix applied via > > >>> the > > >>> plane's CTM property > > >>> "RGB to RGB: Use CTM":* > > >>> Only valid for RGB formats. Use the matrix applied via the > > >>> plane's CTM property > > >>> "Use CTM":* > > >>> Valid for any format. Use the matrix applied via the > > >>> plane's > > >>> CTM property > > >>> ... any other values for BT.601, BT.2020, RGB to YCbCr etc. > > >>> etc. as > > >>> they are required. > > >> Having some RGB2RGB and YCBCR2RGB things in the same property > > >> seems > > >> weird. I would just go with something very simple like: > > >> > > >> YCBCR_TO_RGB_CSC: > > >> * BT.601 > > >> * BT.709 > > >> * custom matrix > > >> > > > I think we've agreed in #dri-devel that this CSC property > > > can't/shouldn't be mapped on-to the existing (hardware > > > implementing > > > the) CTM property - even in the case of per-plane color > > > management - > > > because CSC needs to be done before DEGAMMA. > > > > > > So, I'm in favour of going with what you suggested in the > > > first place: > > > > > > A new YCBCR_TO_RGB_CSC property, enum type, with a list of fixed > > > conversions. I'd drop the custom matrix for now, as we'd need > > > to add > > > another property to attach the custom matrix blob too. > > > > > > I still think we need a way to specify whether
Re: [PATCH] drm: add check for plane functions
On Fri, Mar 17, 2017 at 3:26 PM, Ville Syrjälä
wrote:
> On Fri, Mar 17, 2017 at 01:25:08PM +0530, Shirish S wrote:
>> update_plane() and disable_plane() functions
>> assoiciated with setting plane are called
>> without any check, causing kernel panic.
>
> Why are you registering a plane without the funcs?
>
Basically, enabling planes and making them fully functional is
generally a 2 -step process,
so i suggest for new drivers wanting to implement/re-design planes,
would like to tap
the flow at enabling(listing caps) and later at ensuring it works.
I noticed that there is a underlying assumption only for
plane->(funcs) are implemented, whereas for
other function for crtc/connector/encoder function calls there is a
sanity check(or WARN_ON) through out the framework.
I believe this check wont cause any performance/functional impact.
Please let me know if am missing anything.
And further more help developers to focus on enabling planes via
various tests without causing reboots/system hangs.
>>
>> This patch adds the required check to avoid it.
>>
>> Change-Id: I0d6792608b33e674c217388aa57c4b7d680d9bc7
>> Signed-off-by: Shirish S
>> ---
>> drivers/gpu/drm/drm_plane.c | 6 ++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
>> index 249c0ae..f675f8b 100644
>> --- a/drivers/gpu/drm/drm_plane.c
>> +++ b/drivers/gpu/drm/drm_plane.c
>> @@ -456,6 +456,12 @@ static int __setplane_internal(struct drm_plane *plane,
>> {
>> int ret = 0;
>>
>> + if (plane->funcs->disable_plane == NULL ||
>> + plane->funcs->update_plane == NULL) {
>> + DRM_ERROR("plane funcs not implemented\n");
>> + ret = -EPERM;
>> + goto out;
>> + }
>> /* No fb means shut it down */
>> if (!fb) {
>> plane->old_fb = plane->fb;
>> --
>> 2.7.4
>>
>> ___
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Ville Syrjälä
> Intel OTC
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC libdrm 0/2] Replace the build system with meson
On 03/16/2017 10:57 PM, Dylan Baker wrote: > Quoting Ilia Mirkin (2017-03-16 14:32:09) >> On Thu, Mar 16, 2017 at 5:25 PM, Dylan Baker wrote: >>> Why bother, and why would we want this? >>> │~ >>> >>> First it's written in python, which means the potential developer base >>> is massive. And it provides a recursive view for humans, but a >>> non-recursive view for the system. This is the best of both worlds, >>> humans can organize the build system in a way that makes sense, and the >>> machine gets a non-recursive build system. It also uses ninja rather >>> than make, and ninja is faster than make inherently. Meson is also a >>> simpler syntax than autotools or cmake it's not Turing Complete by >>> design nor does it expose python, again, by design. This allows meson >>> itself to be reimplemented in a another language if python becomes a >>> dead-end or a bottle-neck. It also makes it much easier to understand >>> what the build system is doing. >>> >>> What's different about using meson? >>> >>> Well, apart from a faster builds and less magic in the build system? The >>> configure flags are different, it uses -D= more like cmake >>> than the --enable or --with flags of autotools, although oddly it uses >>> --prefix and friends when calling meson, but not with mesonconf, there's >>> a bug opened on this. Meson also doesn't support in-tree builds at all; >>> all builds are done out of tree. It also doesn't provide a "make dist" >>> target, fortunately there's this awesome tool called git, and it >>> provides a "git archive" command that does much the same thing. Did I >>> mention it's fast? >>> >>> Here are the performance numbers I see on a 2 core 4 thread SKL, without >>> initial configuration, and building out of tree (using zsh): >>> >>> For meson the command line is: >>> time (meson build -Dmanpages=true && ninja -C build) >>> >>> For autotools the command line is: >>> time (mdkir build && cd build && ../autotools && make -j5 -l4) >> >> Probably mkdir... > > derp, yeah. > >> >>> >>> meson (cold ccache): 13.37s user 1.74s system 255% cpu 5.907 total >>> autotools (cold ccache): 26.50s user 1.71s system 129% cpu 21.835 total >>> meson (hot ccache): 2.13s user 0.39s system 154% cpu 1.633 total >>> autotools (hot ccache): 13.93s user 0.73s system 102% cpu 14.259 total >>> >>> That's ~4x faster for a cold build and ~10x faster for a hot build. >>> >>> For a make clean && make style build with a hot cache: >>> meson: 4.64s user 0.33s system 334% cpu 1.486 total >>> autotools: 7.93s user 0.32s system 167% cpu 4.920 total >>> >>> Why bother with libdrm? >>> >>> It's a simple build system, that could be completely (or mostly >>> completely) be ported in a very short time, and could serve as a tech >>> demo for the advantages of using meson to garner feedback for embarking >>> on a larger project, like mesa (which is what I'm planning to work on >>> next). >>> >>> tl;dr >>> >>> I wrote this as practice for porting Mesa, and figured I might as well >>> send it out since I wrote it. >>> >>> It is very likely that neither of these large patches will show up on the >>> mailing list, but this is available at my github: >>> https://github.com/dcbaker/libdrm wip/meson >> >> I haven't looked at meson or your patches in detail, but autotools >> supports 2 very important use-cases very well: >> >> 1. ./configure --help >> 2. Cross-compilation with minimal requirement from the project being built >> >> Can you comment on how these are handled in meson? >> >> Cheers, >> >> -ilia > > 1. mesonconf provides much the same thing. You can also read the > meson_options.txt file, which is generally pretty short. I haven't added > descriptions to the options in this patch. > > 2. you write a small ini style configuration file, something like: > [binaries] > c = '/usr/bin/aarch64-linux-gnu-gc' > ar = '/usr/bin/aarch64-linux-gnu-gcc-ar' > strip = '/usr/bin/aarch64-linux-gnu-strip' > pkg-config = '/usr/bin/aarch64-linux-gnu-pkgconfig' I'm sure meson is a good tool, but a simple : ./configure --target=arch64-linux-gnu is wa simpler... And the only requirement is "sh" for autotools. Neil > > Then you just configure with: > meson build --cross-file cross_file.txt > > then just ninja like normal > > There's a more detailed walkthrough here: > https://github.com/mesonbuild/meson/wiki/Cross-compilation > > I was able to cross compile the arm libraries for aarch64 using basically the > above configuration (I had to write a wrapper script for pkg-config to set a > couple of environment variables and install and archlinux aarach64 chroot, > because arch), of course, I don't have access to any arm machines that I could > test with. > > Dylan > > > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel > signature.asc Descripti
