[PATCH v2 1/2] clk: change clk_ops' ->round_rate() prototype
On 04/30, Boris Brezillon wrote: > Clock rates are stored in an unsigned long field, but ->round_rate() > (which returns a rounded rate from a requested one) returns a long > value (errors are reported using negative error codes), which can lead > to long overflow if the clock rate exceed 2Ghz. > > Change ->round_rate() prototype to return 0 or an error code, and pass the > requested rate as a pointer so that it can be adjusted depending on > hardware capabilities. > > Signed-off-by: Boris Brezillon > Tested-by: Heiko Stuebner > Tested-by: Mikko Perttunen > Reviewed-by: Heiko Stuebner This patch is fairly invasive, and it probably doesn't even matter for most of these clock providers to be able to round a rate above 2GHz. I've been trying to remove the .round_rate op from the framework by encouraging new features via the .determine_rate op. Sadly, we still have to do a flag day and change all the .determine_rate ops when we want to add things. What if we changed determine_rate ops to take a struct clk_determine_info (or some better named structure) instead of the current list of arguments that it currently takes? Then when we want to make these sorts of framework wide changes we can just throw a new member into that structure and be done. It doesn't solve the unsigned long to int return value problem though. We can solve that by gradually introducing a new op and handling another case in the rounding path. If we can come up with some good name for that new op like .decide_rate or something then it makes things nicer in the long run. I like the name .determine_rate though :/ The benefit of all this is that we don't have to worry about finding the random clk providers that get added into other subsystems and fixing them up. If drivers actually care about this problem then they'll be fixed to use the proper op. FYI, last time we updated the function signature of .determine_rate we broke a couple drivers along the way. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
[Bug 89203] Mesa 10.4.3 and up causes stuttering and frame drops in a particular game
https://bugs.freedesktop.org/show_bug.cgi?id=89203 --- Comment #9 from andre35822 at yahoo.com --- The issue seems to no longer occur on Mesa 10.5.4, I am also on kernel 4.0 (I dont remember if it did not occur on Mesa 10.5.4 with kernel 3.19) I guess this is solved? I don't know what was done to solve it but the stuttering no longer occurs and FPS has increased by 10-20FPS. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150506/609ec8b7/attachment.html>
[Bug 97561] Regression: Issue with ATI HD 6570 (possibly others) HDMI output Purple Line
https://bugzilla.kernel.org/show_bug.cgi?id=97561 Rich changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |PATCH_ALREADY_AVAILABLE -- You are receiving this mail because: You are watching the assignee of the bug.
[Bug 97561] Regression: Issue with ATI HD 6570 (possibly others) HDMI output Purple Line
https://bugzilla.kernel.org/show_bug.cgi?id=97561 --- Comment #8 from Rich --- Thanks - I did try to compile my own kernel a few times but failed. I managed to 'git clone' the kernel source, apply the drm-fixes patches successfully, compile (with various config settings) but could never get the kernel to boot - but thats my problem and need to learn! On topic: 4.1-rc2 [4.1.0-040100rc2-generic] fixes the problem. I assume its the same bug as you posted. I am going to resolve this now (if I can!) Thanks! -- You are receiving this mail because: You are watching the assignee of the bug.
[PATCH RFC v2 10/13] sound/core: add DRM ELD helper
05.04.2015, 20:26, Russell King - ARM Linux kirjoitti: > On Sun, Apr 05, 2015 at 06:46:13PM +0200, Takashi Iwai wrote: >> >> One another question: don't we need to deal with the sample bits in >> sad[2]? > > It should, but I'm very wary about doing that without seeing more > examples of real SADs. Right now, all my examples only support > one SAD with either 2 channel or 6 channel audio at the standard > (basic) 32, 44.1 and 48kHz rates. > > The HDMI / CEA specs are very loose in their wording about the > short audio descriptors. I've no idea whether a sink can provide > (for example) descriptors such as: > > LPCM, 6 channel 32, 44.1, 48kHz > LPCM, 2 channel, 32, 44.1, 48, 96, 192kHz > > or whether have to describe that as a single descriptor. I only > have two TVs to test with here. For the record, yes, multiple LPCM SADs are relatively common, and I think I've seen some that offered more rates on a stereo SAD than on a multichannel SAD (like in your example). > What I'm concerned about is that when the ALSA parameter refining > starts, we start with (eg) 2-8 channels, 32-192kHz. Given that, > if we invoke the channel restriction before the rate restriction, > we would end up limiting to 2 channel at 32-192kHz. If we apply > the restrictions in the opposite order, we'd restrict to 6 > channel, 32-48kHz. Neither are obviously correct in this > circumstance, and I don't really see a way to solve it given my > understanding of the way ALSA's parameter refinement works. > > I suspect this is why most HDMI drivers are implemented such that > they take the maximum capabilities over all SADs, which would end > up restricting audio in the above case to: up to 6 channels, at > 32, 44.1, 48, 96 and 192kHz, even though 6 channel @ 192kHz isn't > hasn't been indicated as supported. > > Most of this is speculation though, based off what is in the > documentation. As I say, I don't have enough real-world examples > to get a feel for what manufacturers _actually_ do to give a hint > as to how the documentation should be interpreted. > > So, maybe I should just copy what everyone else does and take the > maximum of all descriptors... > -- Anssi Hannula
component_unbind_all() splat..
On Wed, May 06, 2015 at 12:45:04PM -0400, Rob Clark wrote: > On Wed, May 6, 2015 at 12:01 PM, Russell King - ARM Linux > wrote: > > On Wed, May 06, 2015 at 10:22:22AM -0400, Rob Clark wrote: > >> Hey Russell, > >> > >> I just noticed a splat triggered by component_unbind_all() called from > >> ->bind().. any opinions about whether component stuff should handle > >> that case, or whether I should rearrange my error cleanup path to not > >> component_unbind_all() in this case? > > > > Essentially, if component_bind_all() returned an error, you should not > > call component_unbind_all() - component_bind_all() has the effect that > > it's either successful and all components have been bound, or when it > > fails, no components are bound. > > > > component_unbind_all() assumes that the components were previously > > successfully bound. > > > > When I look at the MSM code, I can see why this happens, and it's > > basically because of broken error cleanup handling caused by this > > commit: > > Right, what to do to fix it in msm is clear enough.. what I was > (trying to) get at was, since error paths are perpetually undertested, > would it make more sense to handle component_unbind_all() called from > ->bind().. (although I also didn't go audit the other component users > yet to see if any might have the same issue) You can call it from ->bind() provided the preceding component_bind_all() call succeeded. -- FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net.
[Bug 90340] RADEON "PITCAIRN" displayport output breaks when monitor turned off and on
https://bugs.freedesktop.org/show_bug.cgi?id=90340 Niels Ole Salscheider changed: What|Removed |Added CC||niels_ole at salscheider-onlin ||e.de --- Comment #1 from Niels Ole Salscheider --- I have seen similar behaviour on a Barts card with an NEC MultiSync PA271W... -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150506/871945ca/attachment.html>
[PATCH 2/2 linux-next] drm/msm: use IS_ERR() to check regulator_get() return
Please ignore this patch, I just noticed conditional regulator_get() definition (#if defined(CONFIG_REGULATOR)) Regards, Fabian
[Bug 25898] glEvalPoint causes glEnd to throw GL_INVALID_OPERATION
https://bugs.freedesktop.org/show_bug.cgi?id=25898 marius predut changed: What|Removed |Added Assignee|dri-devel at lists.freedesktop |marius.predut at intel.com |.org| -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150506/e997418c/attachment.html>
drm/exynos: Add atomic modesetting support
Hi Inki and Joonyoung. Any thoughts on this? 2015-04-30 Gustavo Padovan : > Hi, > > Here goes the full support for atomic modesetting on exynos. I've > split the patches in the various phases of atomic support. > > v2: fixes comments by Joonyoung > - remove unused var in patch 09 > - use ->disable instead of outdated ->dpms in hdmi code > - remove WARN_ON from crtc enable/disable > > v3: fixes comment by Joonyoung > - move the removal of drm_helper_disable_unused_functions() to > separated patch > > v4: add patches that remove unnecessary calls to disable_plane() > > Gustavo > > --- > Gustavo Padovan (12): > drm/exynos: atomic phase 1: use drm_plane_helper_update() > drm/exynos: atomic phase 1: use drm_plane_helper_disable() > drm/exynos: atomic phase 1: add .mode_set_nofb() callback > drm/exynos: atomic phase 2: wire up state reset(), duplicate() and > destroy() > drm/exynos: atomic phase 2: keep track of framebuffer pointer > drm/exynos: atomic phase 3: atomic updates of planes > drm/exynos: atomic phase 3: use atomic .set_config helper > drm/exynos: atomic phase 3: convert page flips > drm/exynos: remove exported functions from exynos_drm_plane > drm/exynos: don't disable unused functions at init > drm/exynos: atomic dpms support > drm/exynos: remove unnecessary calls to disable_plane() > > drivers/gpu/drm/bridge/ps8622.c | 6 +- > drivers/gpu/drm/bridge/ptn3460.c| 6 +- > drivers/gpu/drm/exynos/exynos_dp_core.c | 6 +- > drivers/gpu/drm/exynos/exynos_drm_crtc.c| 215 > +-- > drivers/gpu/drm/exynos/exynos_drm_dpi.c | 6 +- > drivers/gpu/drm/exynos/exynos_drm_drv.c | 2 + > drivers/gpu/drm/exynos/exynos_drm_drv.h | 4 +- > drivers/gpu/drm/exynos/exynos_drm_dsi.c | 6 +- > drivers/gpu/drm/exynos/exynos_drm_encoder.c | 35 + > drivers/gpu/drm/exynos/exynos_drm_fb.c | 12 +- > drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 3 - > drivers/gpu/drm/exynos/exynos_drm_plane.c | 115 + > drivers/gpu/drm/exynos/exynos_drm_plane.h | 11 -- > drivers/gpu/drm/exynos/exynos_drm_vidi.c| 6 +- > drivers/gpu/drm/exynos/exynos_hdmi.c| 10 +- > 15 files changed, 178 insertions(+), 265 deletions(-) > Gustavo
[Y2038] [PATCH v4 06/10] cec: add HDMI CEC framework: y2038 question
On Wednesday 06 May 2015 17:58:01 Hans Verkuil wrote: > > On 05/04/2015 12:14 PM, Arnd Bergmann wrote: > > On Monday 04 May 2015 09:42:36 Hans Verkuil wrote: > >> Ping! (Added Arnd to the CC list) > > > > Hi Hans, > > > > sorry I missed this the first time > > > >> On 04/27/2015 09:40 AM, Hans Verkuil wrote: > >>> Added the y2038 mailinglist since I would like to get their input for > >>> this API. > >>> > >>> Y2038 experts, can you take a look at my comment in the code below? > >>> > >>> Thanks! > >> > >> Arnd, I just saw your patch series adding struct __kernel_timespec to > >> uapi/linux/time.h. I get the feeling that it might take a few kernel > >> cycles before we have a timespec64 available in userspace. Based on that > >> I think this CEC API should drop the timestamps for now and wait until > >> timespec64 becomes available before adding it. > >> > >> The timestamps are a nice-to-have, but not critical. So adding it later > >> shouldn't be a problem. What is your opinion? > > > > It will take a little while for the patches to make it in, I would guess > > 4.3 at the earliest. Using your own struct works just as well and would > > be less ambiguous. > > > > However, for timestamps, I would recommend not using timespec anyway. > > Instead, just use a single 64-bit nanosecond value from ktime_get_ns() > > (or ktime_get_boot_ns() if you need a time that keeps ticking across > > suspend). This is more efficient to get and simpler to use as long > > as you don't need to convert from nanosecond to timespec. > > Possibly stupid follow-up question: > > is ktime_get_ns() just a different representation as ktime_get_ts64()? Yes. > Or is there some offset between the two? They seem to be identical based > on a quick test, but I'd like to be certain that that's always the case. > > Users need to be able to relate this timestamp to a struct timespec as > returned by V4L2 (and others). * ktime_get_ns() uses the same timebase as ktime_get_ts64(). * ktime_get_boot_ns() uses the same timebase as ktime_get_boottime() or getboottime64(), which differs from the first after suspend * ktime_get_real_ns() uses the same time as gettimeofday() in user space, which is always different from the other two. Arnd
[Y2038] [PATCH v4 06/10] cec: add HDMI CEC framework: y2038 question
Hi Arnd, On 05/04/2015 12:14 PM, Arnd Bergmann wrote: > On Monday 04 May 2015 09:42:36 Hans Verkuil wrote: >> Ping! (Added Arnd to the CC list) > > Hi Hans, > > sorry I missed this the first time > >> On 04/27/2015 09:40 AM, Hans Verkuil wrote: >>> Added the y2038 mailinglist since I would like to get their input for >>> this API. >>> >>> Y2038 experts, can you take a look at my comment in the code below? >>> >>> Thanks! >> >> Arnd, I just saw your patch series adding struct __kernel_timespec to >> uapi/linux/time.h. I get the feeling that it might take a few kernel >> cycles before we have a timespec64 available in userspace. Based on that >> I think this CEC API should drop the timestamps for now and wait until >> timespec64 becomes available before adding it. >> >> The timestamps are a nice-to-have, but not critical. So adding it later >> shouldn't be a problem. What is your opinion? > > It will take a little while for the patches to make it in, I would guess > 4.3 at the earliest. Using your own struct works just as well and would > be less ambiguous. > > However, for timestamps, I would recommend not using timespec anyway. > Instead, just use a single 64-bit nanosecond value from ktime_get_ns() > (or ktime_get_boot_ns() if you need a time that keeps ticking across > suspend). This is more efficient to get and simpler to use as long > as you don't need to convert from nanosecond to timespec. Possibly stupid follow-up question: is ktime_get_ns() just a different representation as ktime_get_ts64()? Or is there some offset between the two? They seem to be identical based on a quick test, but I'd like to be certain that that's always the case. Users need to be able to relate this timestamp to a struct timespec as returned by V4L2 (and others). Thanks! Hans
[Intel-gfx] [PATCH 1/8] drivers/gpio/gpiolib: Add support for removing registered consumer lookup table
On Wed, May 06, 2015 at 04:49:36PM +0200, Linus Walleij wrote: > On Wed, Apr 29, 2015 at 3:59 PM, Shobhit Kumar > wrote: > > > In case we unload and load a driver module again that is registering a > > lookup table, without this it will result in multiple entries. Provide > > an option to remove the lookup table on driver unload > > > > v2: Ccing maintainers > > > > Cc: Samuel Ortiz > > Cc: Linus Walleij > > Cc: Alexandre Courbot > > Cc: Thierry Reding > > Reviewed-by: Alexandre Courbot > > Signed-off-by: Shobhit Kumar > > Reviewed-by: Linus Walleij I think the gpio/pwm and mfd parts would all need to go in through the same tree. i915 parts are decoupled. I guess I could do a branch with just those patches, tag it and then send a pull request to all 3 subsystems once it's reviewed. Would that be ok? -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCH 9/9] drm/exynos: Convert g2d_userptr_get_dma_addr() to use get_vaddr_frames()
On Wed 06-05-15 12:47:35, Vlastimil Babka wrote:
> On 05/06/2015 09:28 AM, Jan Kara wrote:
> >Convert g2d_userptr_get_dma_addr() to pin pages using get_vaddr_frames().
> >This removes the knowledge about vmas and mmap_sem locking from exynos
> >driver. Also it fixes a problem that the function has been mapping user
> >provided address without holding mmap_sem.
> >
> >Signed-off-by: Jan Kara
> >---
> > drivers/gpu/drm/exynos/exynos_drm_g2d.c | 89 ++
> > drivers/gpu/drm/exynos/exynos_drm_gem.c | 97
> > -
> > 2 files changed, 29 insertions(+), 157 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/exynos/exynos_drm_g2d.c
> >b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
> >index 81a250830808..265519c0fe2d 100644
> >--- a/drivers/gpu/drm/exynos/exynos_drm_g2d.c
> >+++ b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
> ...
> >@@ -456,65 +458,37 @@ static dma_addr_t *g2d_userptr_get_dma_addr(struct
> >drm_device *drm_dev,
> > return ERR_PTR(-ENOMEM);
> >
> > atomic_set(&g2d_userptr->refcount, 1);
> >+g2d_userptr->size = size;
> >
> > start = userptr & PAGE_MASK;
> > offset = userptr & ~PAGE_MASK;
> > end = PAGE_ALIGN(userptr + size);
> > npages = (end - start) >> PAGE_SHIFT;
> >-g2d_userptr->npages = npages;
> >-
> >-pages = drm_calloc_large(npages, sizeof(struct page *));
> >-if (!pages) {
> >-DRM_ERROR("failed to allocate pages.\n");
> >-ret = -ENOMEM;
> >+vec = g2d_userptr->vec = frame_vector_create(npages);
> >+if (!vec)
> > goto err_free;
> >-}
> >
> >-down_read(¤t->mm->mmap_sem);
> >-vma = find_vma(current->mm, userptr);
> >-if (!vma) {
> >-up_read(¤t->mm->mmap_sem);
> >-DRM_ERROR("failed to get vm region.\n");
> >+ret = get_vaddr_frames(start, npages, 1, 1, vec);
>
> Use true instead of 1.
Yes, thanks!
Honza
--
Jan Kara
SUSE Labs, CR
[PATCH 3/9] media: omap_vout: Convert omap_vout_uservirt_to_phys() to use get_vaddr_pfns()
On Wed 06-05-15 12:46:42, Vlastimil Babka wrote:
> On 05/06/2015 09:28 AM, Jan Kara wrote:
> >Convert omap_vout_uservirt_to_phys() to use get_vaddr_pfns() instead of
> >hand made mapping of virtual address to physical address. Also the
> >function leaked page reference from get_user_pages() so fix that by
> >properly release the reference when omap_vout_buffer_release() is
> >called.
> >
> >Signed-off-by: Jan Kara
> >---
> > drivers/media/platform/omap/omap_vout.c | 67
> > +++--
> > 1 file changed, 31 insertions(+), 36 deletions(-)
> >
>
> ...
>
> >+vec = frame_vector_create(1);
> >+if (!vec)
> >+return -ENOMEM;
> >
> >-if (res == nr_pages) {
> >-physp = __pa(page_address(&pages[0]) +
> >-(virtp & ~PAGE_MASK));
> >-} else {
> >-printk(KERN_WARNING VOUT_NAME
> >-"get_user_pages failed\n");
> >-return 0;
> >-}
> >+ret = get_vaddr_frames(virtp, 1, 1, 0, vec);
>
> Use true/false where appropriate.
Right. Thanks.
Honza
--
Jan Kara
SUSE Labs, CR
component_unbind_all() splat..
On Wed, May 06, 2015 at 10:22:22AM -0400, Rob Clark wrote:
> Hey Russell,
>
> I just noticed a splat triggered by component_unbind_all() called from
> ->bind().. any opinions about whether component stuff should handle
> that case, or whether I should rearrange my error cleanup path to not
> component_unbind_all() in this case?
Essentially, if component_bind_all() returned an error, you should not
call component_unbind_all() - component_bind_all() has the effect that
it's either successful and all components have been bound, or when it
fails, no components are bound.
component_unbind_all() assumes that the components were previously
successfully bound.
When I look at the MSM code, I can see why this happens, and it's
basically because of broken error cleanup handling caused by this
commit:
commit 5bf9c0b614542d69fb9a8681a0411715cc3e8ba8
Author: Rob Clark
Date: Tue Mar 3 15:04:24 2015 -0500
drm/msm: split out vram initialization
We'll want to extend this a bit to handle also a reserved-memory
("stolen") region, so that drm/msm can take-over bootloader splash
screen. First split it out into it's own fxn to reduce noise in
the following patch.
Signed-off-by: Rob Clark
Let's look at the clean-up paths:
priv = kzalloc(sizeof(*priv), GFP_KERNEL);
if (!priv) {
dev_err(dev->dev, "failed to allocate private data\n");
return -ENOMEM;
returns immediately.
ret = msm_init_vram(dev);
if (ret)
goto fail;
calls msm_unload.
/* Bind all our sub-components: */
ret = component_bind_all(dev->dev, dev);
if (ret)
return ret;
doesn't call msm_unload.
Why would the second not need to run any cleanup if failing at
msm_init_vram() does? This is totally mad.
I think you need to fix this driver to have a sane error cleanup
implementation...
--
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
[PATCH 2/9] mm: Provide new get_vaddr_frames() helper
On Wed 06-05-15 12:45:51, Vlastimil Babka wrote:
> On 05/06/2015 09:28 AM, Jan Kara wrote:
> >Provide new function get_vaddr_frames(). This function maps virtual
> >addresses from given start and fills given array with page frame numbers of
> >the corresponding pages. If given start belongs to a normal vma, the function
> >grabs reference to each of the pages to pin them in memory. If start
> >belongs to VM_IO | VM_PFNMAP vma, we don't touch page structures. Caller
> >must make sure pfns aren't reused for anything else while he is using
> >them.
> >
> >This function is created for various drivers to simplify handling of
> >their buffers.
> >
> >Signed-off-by: Jan Kara
>
> Acked-by: Vlastimil Babka
>
> With some nitpicks below...
>
> >---
> > include/linux/mm.h | 44 +++
> > mm/gup.c | 214
> > +
> > 2 files changed, 258 insertions(+)
> >
> >diff --git a/include/linux/mm.h b/include/linux/mm.h
> >index 0755b9fd03a7..dcd1f02a78e9 100644
> >--- a/include/linux/mm.h
> >+++ b/include/linux/mm.h
> >@@ -20,6 +20,7 @@
> > #include
> > #include
> > #include
> >+#include
> >
> > struct mempolicy;
> > struct anon_vma;
> >@@ -1197,6 +1198,49 @@ long get_user_pages_unlocked(struct task_struct *tsk,
> >struct mm_struct *mm,
> > int write, int force, struct page **pages);
> > int get_user_pages_fast(unsigned long start, int nr_pages, int write,
> > struct page **pages);
> >+
> >+/* Container for pinned pfns / pages */
> >+struct frame_vector {
> >+unsigned int nr_allocated; /* Number of frames we have space for */
> >+unsigned int nr_frames; /* Number of frames stored in ptrs array */
> >+bool got_ref; /* Did we pin pages by getting page ref? */
> >+bool is_pfns; /* Does array contain pages or pfns? */
> >+void *ptrs[0]; /* Array of pinned pfns / pages. Use
> >+ * pfns_vector_pages() or pfns_vector_pfns()
> >+ * for access */
> >+};
> >+
> >+struct frame_vector *frame_vector_create(unsigned int nr_frames);
> >+void frame_vector_destroy(struct frame_vector *vec);
> >+int get_vaddr_frames(unsigned long start, unsigned int nr_pfns,
> >+ bool write, bool force, struct frame_vector *vec);
> >+void put_vaddr_frames(struct frame_vector *vec);
> >+int frame_vector_to_pages(struct frame_vector *vec);
> >+void frame_vector_to_pfns(struct frame_vector *vec);
> >+
> >+static inline unsigned int frame_vector_count(struct frame_vector *vec)
> >+{
> >+return vec->nr_frames;
> >+}
> >+
> >+static inline struct page **frame_vector_pages(struct frame_vector *vec)
> >+{
> >+if (vec->is_pfns) {
> >+int err = frame_vector_to_pages(vec);
> >+
> >+if (err)
> >+return ERR_PTR(err);
> >+}
> >+return (struct page **)(vec->ptrs);
> >+}
> >+
> >+static inline unsigned long *frame_vector_pfns(struct frame_vector *vec)
> >+{
> >+if (!vec->is_pfns)
> >+frame_vector_to_pfns(vec);
> >+return (unsigned long *)(vec->ptrs);
> >+}
> >+
> > struct kvec;
> > int get_kernel_pages(const struct kvec *iov, int nr_pages, int write,
> > struct page **pages);
> >diff --git a/mm/gup.c b/mm/gup.c
> >index 6297f6bccfb1..8db5c40e65c4 100644
> >--- a/mm/gup.c
> >+++ b/mm/gup.c
> >@@ -8,6 +8,7 @@
> > #include
> > #include
> > #include
> >+#include
> >
> > #include
> > #include
> >@@ -936,6 +937,219 @@ int __mm_populate(unsigned long start, unsigned long
> >len, int ignore_errors)
> > return ret; /* 0 or negative error code */
> > }
> >
> >+/*
> >+ * get_vaddr_frames() - map virtual addresses to pfns
> >+ * @start: starting user address
> >+ * @nr_frames: number of pages / pfns from start to map
> >+ * @write: whether pages will be written to by the caller
> >+ * @force: whether to force write access even if user mapping is
> >+ * readonly. This will result in the page being COWed even
> >+ * in MAP_SHARED mappings. You do not want this.
>
> "You do not want this" and yet some of the drivers in later patches
> do. That looks weird. Explain better?
That was copied from get_user_pages() comment but that got changed in the
mean time. I've referenced that instead of copying it.
> >+ * @vec:structure which receives pages / pfns of the addresses mapped.
> >+ * It should have space for at least nr_frames entries.
> >+ *
> >+ * This function maps virtual addresses from @start and fills @vec structure
> >+ * with page frame numbers or page pointers to corresponding pages (choice
> >+ * depends on the type of the vma underlying the virtual address). If @start
> >+ * belongs to a normal vma, the function grabs reference to each of the
> >pages
> >+ * to pin them in memory. If @start belongs to VM_IO | VM_PFNMAP vma, we
> >don't
> >+ * touch page structures and the caller
[PATCH 3/8] drivers/mfd: Add lookup table for Panel Control as GPIO signal
On Wed, Apr 29, 2015 at 4:00 PM, Shobhit Kumar wrote: > On some Intel SoC platforms, the panel enable/disable signals are > controlled by CRC PMIC. Add those control as a new GPIO in a lookup > table for gpio-crystalcove chip during CRC driver load > > v2: Make the lookup table static (Thierry) > Remove the lookup table during driver remove (Thierry) > > CC: Samuel Ortiz > Cc: Linus Walleij > Cc: Alexandre Courbot > Cc: Thierry Reding > Signed-off-by: Shobhit Kumar Acked-by: Linus Walleij Yours, Linus Walleij
[PATCH 1/8] drivers/gpio/gpiolib: Add support for removing registered consumer lookup table
On Wed, Apr 29, 2015 at 3:59 PM, Shobhit Kumar wrote: > In case we unload and load a driver module again that is registering a > lookup table, without this it will result in multiple entries. Provide > an option to remove the lookup table on driver unload > > v2: Ccing maintainers > > Cc: Samuel Ortiz > Cc: Linus Walleij > Cc: Alexandre Courbot > Cc: Thierry Reding > Reviewed-by: Alexandre Courbot > Signed-off-by: Shobhit Kumar Reviewed-by: Linus Walleij Since it is core support I'd prefer to merge this into the GPIO git tree. Is it OK or do you need to merge it through some other means? Yours, Linus Walleij
[PATCH 1/3] drm/edid: Fix up DMT modes
On Wed, May 06, 2015 at 09:39:13AM -0400, Alex Deucher wrote: > On Wed, May 6, 2015 at 9:36 AM, Ville Syrjälä > wrote: > > On Thu, Apr 02, 2015 at 05:02:09PM +0300, ville.syrjala at linux.intel.com > > wrote: > >> From: Ville Syrjälä > >> > >> Liu Lei noticed that our 1856x1392 at 75Hz DMT mode doesn't match the spec. > >> Fix that up, and also fix up a few other inconsistencies I discovered > >> by parsing the spec (DMT version 1.0, revision 13) and comparing the > >> results to our current DMT mode table. > >> > >> Also clean up the indentation mess for the 1024x768i mode. > >> > >> Cc: "liu,lei" > >> Signed-off-by: Ville Syrjälä > > > > Ping. Anyone interested in DMT modes that match the spec? > > I started reviewing this series when you sent it out, but I guess I > got distracted and never sent out my rb: > > Reviewed-by: Alex Deucher Pulled into topic/drm-misc, thanks. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCH 8/8] drm/i915: Backlight control using CRC PMIC based PWM driver
On Wed, 29 Apr 2015, Shobhit Kumar wrote:
> Use the CRC PWM device in intel_panel.c and add new MIPI backlight
> specififc callbacks
>
> v2: Modify to use pwm_config callback
>
> CC: Samuel Ortiz
> Cc: Linus Walleij
> Cc: Alexandre Courbot
> Cc: Thierry Reding
> Signed-off-by: Shobhit Kumar
> ---
> drivers/gpu/drm/i915/intel_drv.h | 5 +++
> drivers/gpu/drm/i915/intel_dsi.c | 6 +++
> drivers/gpu/drm/i915/intel_panel.c | 92
> +++---
> 3 files changed, 98 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 897f17d..b4ebe3b 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -182,7 +182,12 @@ struct intel_panel {
> bool enabled;
> bool combination_mode; /* gen 2/4 only */
> bool active_low_pwm;
> +
> + /* PWM chip */
> + struct pwm_device *pwm;
> +
> struct backlight_device *device;
> +
Superfluous whitespace.
> } backlight;
>
> void (*backlight_power)(struct intel_connector *, bool enable);
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c
> b/drivers/gpu/drm/i915/intel_dsi.c
> index be55ffa..83c4540 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -402,6 +402,8 @@ static void intel_dsi_enable(struct intel_encoder
> *encoder)
>
> intel_dsi_port_enable(encoder);
> }
> +
> + intel_panel_enable_backlight(intel_dsi->attached_connector);
> }
>
> static void intel_dsi_pre_enable(struct intel_encoder *encoder)
> @@ -466,6 +468,8 @@ static void intel_dsi_pre_disable(struct intel_encoder
> *encoder)
>
> DRM_DEBUG_KMS("\n");
>
> + intel_panel_disable_backlight(intel_dsi->attached_connector);
> +
> if (is_vid_mode(intel_dsi)) {
> /* Send Shutdown command to the panel in LP mode */
> for_each_dsi_port(port, intel_dsi->ports)
> @@ -1132,6 +1136,8 @@ void intel_dsi_init(struct drm_device *dev)
> }
>
> intel_panel_init(&intel_connector->panel, fixed_mode, NULL);
> + intel_panel_setup_backlight(connector,
> + (intel_encoder->crtc_mask = (1 << PIPE_A)) ? PIPE_A: PIPE_B);
>
> return;
>
> diff --git a/drivers/gpu/drm/i915/intel_panel.c
> b/drivers/gpu/drm/i915/intel_panel.c
> index 08532d4..5700f6f 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -32,8 +32,12 @@
>
> #include
> #include
> +#include
> #include "intel_drv.h"
>
> +#define CRC_PMIC_PWM_PERIOD_NS 21333
> +#define CRC_PMIC_PWM_STEPS 255
> +
> void
> intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode,
> struct drm_display_mode *adjusted_mode)
> @@ -536,6 +540,15 @@ static u32 vlv_get_backlight(struct intel_connector
> *connector)
> return _vlv_get_backlight(dev, pipe);
> }
>
> +static u32 vlv_get_mipi_backlight(struct intel_connector *connector)
I'd rather call this either pmic_get_backlight or even pwm_get_backlight
because there's nothing really vlv specific about the functions
themselves. Same for all of them.
> +{
> + struct intel_panel *panel = &connector->panel;
> + int duty_ns;
> +
> + duty_ns = pwm_get_duty_cycle(panel->backlight.pwm);
> + return DIV_ROUND_UP(duty_ns * 100, CRC_PMIC_PWM_PERIOD_NS);
Basically we have all the code in place to do the scaling from hw to
user. See the note in intel_backlight_device_register. We shouldn't need
to add additional scaling all around.
However this can be a future cleanup IMO.
> +}
> +
> static u32 intel_panel_get_backlight(struct intel_connector *connector)
> {
> struct drm_device *dev = connector->base.dev;
> @@ -616,6 +629,14 @@ static void vlv_set_backlight(struct intel_connector
> *connector, u32 level)
> I915_WRITE(VLV_BLC_PWM_CTL(pipe), tmp | level);
> }
>
> +static void vlv_set_mipi_backlight(struct intel_connector *connector, u32
> level)
> +{
> + struct intel_panel *panel = &connector->panel;
> + int duty_ns = DIV_ROUND_UP(level * CRC_PMIC_PWM_PERIOD_NS, 100);
> +
> + pwm_config(panel->backlight.pwm, duty_ns, CRC_PMIC_PWM_PERIOD_NS);
> +}
> +
> static void
> intel_panel_actually_set_backlight(struct intel_connector *connector, u32
> level)
> {
> @@ -741,6 +762,16 @@ static void vlv_disable_backlight(struct intel_connector
> *connector)
> I915_WRITE(VLV_BLC_PWM_CTL2(pipe), tmp & ~BLM_PWM_ENABLE);
> }
>
> +static void vlv_disable_mipi_backlight(struct intel_connector *connector)
> +{
> + struct intel_panel *panel = &connector->panel;
> +
> + /* Disable the backlight */
> + pwm_config(panel->backlight.pwm, 0, CRC_PMIC_PWM_PERIOD_NS);
> + usleep_range(2000, 3000);
> + pwm_disable(panel->backlight.pwm);
> +}
> +
> void intel_panel_disable_backlight(struct intel_connector *connector)
> {
> struc
[Bug 95911] Recursive error in radeon device driver module after resume from hibernation
https://bugzilla.kernel.org/show_bug.cgi?id=95911 --- Comment #13 from gitne at excite.co.jp --- This bug seems to be linked to bugs bz77181 and bz60827 too. Mantas MikulÄnas has determined that git commit 4474f3a91f95 was the last known good to work. archiesix has determined that this bug persists even since kernel version 3.9.11. It's a pity that actually *users* have to do the digging for this kind of information. Its all there but kernel developers are obviously too tired or too lazy to do actual work after they have spent countless hours bragging about how genius they are in delivering fucked up work. If you can't do it, don't touch it. Oh and another "secret" has been revealed: The bug is caused by ring test failures. Wow! Who could have thought of that! @Michel Dänzer To answer your previous question: Yes, the bug is a regression, or sort of. -- You are receiving this mail because: You are watching the assignee of the bug.
[PATCH 1/3] drm/edid: Fix up DMT modes
On Thu, Apr 02, 2015 at 05:02:09PM +0300, ville.syrjala at linux.intel.com
wrote:
> From: Ville Syrjälä
>
> Liu Lei noticed that our 1856x1392 at 75Hz DMT mode doesn't match the spec.
> Fix that up, and also fix up a few other inconsistencies I discovered
> by parsing the spec (DMT version 1.0, revision 13) and comparing the
> results to our current DMT mode table.
>
> Also clean up the indentation mess for the 1024x768i mode.
>
> Cc: "liu,lei"
> Signed-off-by: Ville Syrjälä
Ping. Anyone interested in DMT modes that match the spec?
> ---
> drivers/gpu/drm/drm_edid.c | 10 +-
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 53bc7a6..567c7ed 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -160,7 +160,7 @@ static const struct drm_display_mode drm_dmt_modes[] = {
> DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_PVSYNC) },
> /* 640x480 at 60Hz */
> { DRM_MODE("640x480", DRM_MODE_TYPE_DRIVER, 25175, 640, 656,
> -752, 800, 0, 480, 489, 492, 525, 0,
> +752, 800, 0, 480, 490, 492, 525, 0,
> DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC) },
> /* 640x480 at 72Hz */
> { DRM_MODE("640x480", DRM_MODE_TYPE_DRIVER, 31500, 640, 664,
> @@ -206,7 +206,7 @@ static const struct drm_display_mode drm_dmt_modes[] = {
> { DRM_MODE("1024x768i", DRM_MODE_TYPE_DRIVER, 44900, 1024, 1032,
> 1208, 1264, 0, 768, 768, 772, 817, 0,
> DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC |
> - DRM_MODE_FLAG_INTERLACE) },
> +DRM_MODE_FLAG_INTERLACE) },
> /* 1024x768 at 60Hz */
> { DRM_MODE("1024x768", DRM_MODE_TYPE_DRIVER, 65000, 1024, 1048,
> 1184, 1344, 0, 768, 771, 777, 806, 0,
> @@ -242,7 +242,7 @@ static const struct drm_display_mode drm_dmt_modes[] = {
> /* 1280x768 at 75Hz */
> { DRM_MODE("1280x768", DRM_MODE_TYPE_DRIVER, 102250, 1280, 1360,
> 1488, 1696, 0, 768, 771, 778, 805, 0,
> -DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NVSYNC) },
> +DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_PVSYNC) },
> /* 1280x768 at 85Hz */
> { DRM_MODE("1280x768", DRM_MODE_TYPE_DRIVER, 117500, 1280, 1360,
> 1496, 1712, 0, 768, 771, 778, 809, 0,
> @@ -258,7 +258,7 @@ static const struct drm_display_mode drm_dmt_modes[] = {
> /* 1280x800 at 60Hz */
> { DRM_MODE("1280x800", DRM_MODE_TYPE_DRIVER, 83500, 1280, 1352,
> 1480, 1680, 0, 800, 803, 809, 831, 0,
> -DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NVSYNC) },
> +DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_PVSYNC) },
> /* 1280x800 at 75Hz */
> { DRM_MODE("1280x800", DRM_MODE_TYPE_DRIVER, 106500, 1280, 1360,
> 1488, 1696, 0, 800, 803, 809, 838, 0,
> @@ -409,7 +409,7 @@ static const struct drm_display_mode drm_dmt_modes[] = {
> DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_PVSYNC) },
> /* 1856x1392 at 75Hz */
> { DRM_MODE("1856x1392", DRM_MODE_TYPE_DRIVER, 288000, 1856, 1984,
> -2208, 2560, 0, 1392, 1395, 1399, 1500, 0,
> +2208, 2560, 0, 1392, 1393, 1396, 1500, 0,
> DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_PVSYNC) },
> /* 1856x1392 at 120Hz RB */
> { DRM_MODE("1856x1392", DRM_MODE_TYPE_DRIVER, 356500, 1856, 1904,
> --
> 2.0.5
--
Ville Syrjälä
Intel OTC
[PATCH 7/8] drm/i915: Use the CRC gpio for panel enable/disable
On Wed, 29 Apr 2015, Shobhit Kumar wrote:
> The CRC (Crystal Cove) PMIC, controls the panel enable and disable
> signals for BYT for dsi panels. This is indicated in the VBT fields. Use
> that to initialize and use GPIO based control for these signals.
>
> v2: Use the newer gpiod interface(Alexandre)
> v3: Remove the redundant checks and unused code (Ville)
>
> CC: Samuel Ortiz
> Cc: Linus Walleij
> Cc: Alexandre Courbot
> Cc: Thierry Reding
> Signed-off-by: Shobhit Kumar
> ---
> drivers/gpu/drm/i915/intel_dsi.c | 32 ++--
> drivers/gpu/drm/i915/intel_dsi.h | 6 ++
> 2 files changed, 36 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c
> b/drivers/gpu/drm/i915/intel_dsi.c
> index 5196642..be55ffa 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -31,6 +31,7 @@
> #include
> #include
> #include
> +#include
> #include "i915_drv.h"
> #include "intel_drv.h"
> #include "intel_dsi.h"
> @@ -415,6 +416,12 @@ static void intel_dsi_pre_enable(struct intel_encoder
> *encoder)
>
> DRM_DEBUG_KMS("\n");
>
> + /* Panel Enable over CRC PMIC */
> + if (intel_dsi->gpio_panel)
> + gpiod_set_value_cansleep(intel_dsi->gpio_panel, 1);
> +
> + msleep(intel_dsi->panel_on_delay);
> +
> /* Disable DPOunit clock gating, can stall pipe
>* and we need DPLL REFA always enabled */
> tmp = I915_READ(DPLL(pipe));
> @@ -432,8 +439,6 @@ static void intel_dsi_pre_enable(struct intel_encoder
> *encoder)
> /* put device in ready state */
> intel_dsi_device_ready(encoder);
>
> - msleep(intel_dsi->panel_on_delay);
> -
> drm_panel_prepare(intel_dsi->panel);
>
> for_each_dsi_port(port, intel_dsi->ports)
> @@ -576,6 +581,10 @@ static void intel_dsi_post_disable(struct intel_encoder
> *encoder)
>
> msleep(intel_dsi->panel_off_delay);
> msleep(intel_dsi->panel_pwr_cycle_delay);
> +
> + /* Panel Disable over CRC PMIC */
> + if (intel_dsi->gpio_panel)
> + gpiod_set_value_cansleep(intel_dsi->gpio_panel, 0);
> }
>
> static bool intel_dsi_get_hw_state(struct intel_encoder *encoder,
> @@ -955,6 +964,11 @@ static void intel_dsi_encoder_destroy(struct drm_encoder
> *encoder)
> /* XXX: Logically this call belongs in the panel driver. */
> drm_panel_remove(intel_dsi->panel);
> }
> +
> + /* dispose of the gpios */
> + if (intel_dsi->gpio_panel)
> + gpiod_put(intel_dsi->gpio_panel);
> +
> intel_encoder_destroy(encoder);
> }
>
> @@ -1071,6 +1085,20 @@ void intel_dsi_init(struct drm_device *dev)
> goto err;
> }
>
> + /*
> + * In case of BYT with CRC PMIC, we need to use GPIO for
> + * Panel control.
> + */
> + if (dev_priv->vbt.dsi.config->pwm_blc == PPS_BLC_PMIC) {
> + intel_dsi->gpio_panel =
> + gpiod_get(dev->dev, "panel", GPIOD_OUT_HIGH);
> +
> + if (IS_ERR(intel_dsi->gpio_panel)) {
> + DRM_ERROR("Failed to own gpio for panel control\n");
> + intel_dsi->gpio_panel = NULL;
> + }
> + }
> +
> intel_encoder->type = INTEL_OUTPUT_DSI;
> intel_encoder->cloneable = 0;
> drm_connector_init(dev, connector, &intel_dsi_connector_funcs,
> diff --git a/drivers/gpu/drm/i915/intel_dsi.h
> b/drivers/gpu/drm/i915/intel_dsi.h
> index 2784ac4..bf1bade 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.h
> +++ b/drivers/gpu/drm/i915/intel_dsi.h
> @@ -29,6 +29,9 @@
> #include
> #include "intel_drv.h"
>
> +#define PPS_BLC_PMIC 0
> +#define PPS_BLC_SOC1
Since these values are defined in the VBT, perhaps these should be in
intel_bios.h. Up to you, I'm fine either way.
Reviewed-by: Jani Nikula
> +
> /* Dual Link support */
> #define DSI_DUAL_LINK_NONE 0
> #define DSI_DUAL_LINK_FRONT_BACK 1
> @@ -42,6 +45,9 @@ struct intel_dsi {
> struct drm_panel *panel;
> struct intel_dsi_host *dsi_hosts[I915_MAX_PORTS];
>
> + /* GPIO Desc for CRC based Panel control */
> + struct gpio_desc *gpio_panel;
> +
> struct intel_connector *attached_connector;
>
> /* bit mask of ports being driven */
> --
> 2.1.0
>
--
Jani Nikula, Intel Open Source Technology Center
[PATCH v3] libgencec: Add userspace library for the generic CEC kernel interface
Hi Kamil, There are a couple of bits that I've missed out previously. All but the missing install of libgencec.pc are general cleanups. On 6 May 2015 at 13:37, Kamil Debski wrote: > --- /dev/null > +++ b/.gitignore > @@ -0,0 +1,26 @@ > +/ar-lib You can drop this (see the configure.ac note). > +/aclocal.m4 > +/autom4te.cache/* > +/config.* > +/configure > +/depcomp > +/install-sh > +/missing Normally one can drop the leading forward slash in .gitignore files. This way git won't bother if you build in a subdirectory - for example $(project_top)/build. > +/examples/.deps/* > +/examples/.libs/* > +src/.deps/* > +src/.libs/* One can replace these four with .deps/ .libs/ > +/examples/cectest echo cectest > examples/.gitignore > --- /dev/null > +++ b/Makefile.am > @@ -0,0 +1,4 @@ > +SUBDIRS = src examples > +ACLOCAL_AMFLAGS = -I m4 > +library_includedir=$(includedir) > +library_include_HEADERS = include/gencec.h We want to install the pc file. Otherwise one cannot really use it. pkgconfigdir = $(libdir)/pkgconfig pkgconfig_DATA = libgencec.pc > --- /dev/null > +++ b/configure.ac > @@ -0,0 +1,27 @@ > +AC_PREREQ(2.60) > + > +AC_INIT([libgencec], [0.1], [k.debski at samsung.com]) > +AM_INIT_AUTOMAKE([-Wall -Werror foreign]) > + > +AC_PROG_CC > +AM_PROG_AR There is not plan to use the library on Windows is there ? If so we can remove this as per the manual [1]. "You must use this macro when you use the archiver in your project, if you want support for unusual archivers such as Microsoft lib" Cheers, Emil [1] http://www.gnu.org/software/automake/manual/html_node/Public-Macros.html
[Intel-gfx] [PATCH 7/8] drm/i915: Use the CRC gpio for panel enable/disable
On Wed, May 06, 2015 at 04:11:11PM +0300, Jani Nikula wrote:
> On Wed, 29 Apr 2015, Shobhit Kumar wrote:
> > The CRC (Crystal Cove) PMIC, controls the panel enable and disable
> > signals for BYT for dsi panels. This is indicated in the VBT fields. Use
> > that to initialize and use GPIO based control for these signals.
> >
> > v2: Use the newer gpiod interface(Alexandre)
> > v3: Remove the redundant checks and unused code (Ville)
> >
> > CC: Samuel Ortiz
> > Cc: Linus Walleij
> > Cc: Alexandre Courbot
> > Cc: Thierry Reding
> > Signed-off-by: Shobhit Kumar
> > ---
> > drivers/gpu/drm/i915/intel_dsi.c | 32 ++--
> > drivers/gpu/drm/i915/intel_dsi.h | 6 ++
> > 2 files changed, 36 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dsi.c
> > b/drivers/gpu/drm/i915/intel_dsi.c
> > index 5196642..be55ffa 100644
> > --- a/drivers/gpu/drm/i915/intel_dsi.c
> > +++ b/drivers/gpu/drm/i915/intel_dsi.c
> > @@ -31,6 +31,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include "i915_drv.h"
> > #include "intel_drv.h"
> > #include "intel_dsi.h"
> > @@ -415,6 +416,12 @@ static void intel_dsi_pre_enable(struct intel_encoder
> > *encoder)
> >
> > DRM_DEBUG_KMS("\n");
> >
> > + /* Panel Enable over CRC PMIC */
> > + if (intel_dsi->gpio_panel)
> > + gpiod_set_value_cansleep(intel_dsi->gpio_panel, 1);
> > +
> > + msleep(intel_dsi->panel_on_delay);
> > +
> > /* Disable DPOunit clock gating, can stall pipe
> > * and we need DPLL REFA always enabled */
> > tmp = I915_READ(DPLL(pipe));
> > @@ -432,8 +439,6 @@ static void intel_dsi_pre_enable(struct intel_encoder
> > *encoder)
> > /* put device in ready state */
> > intel_dsi_device_ready(encoder);
> >
> > - msleep(intel_dsi->panel_on_delay);
> > -
> > drm_panel_prepare(intel_dsi->panel);
> >
> > for_each_dsi_port(port, intel_dsi->ports)
> > @@ -576,6 +581,10 @@ static void intel_dsi_post_disable(struct
> > intel_encoder *encoder)
> >
> > msleep(intel_dsi->panel_off_delay);
> > msleep(intel_dsi->panel_pwr_cycle_delay);
> > +
> > + /* Panel Disable over CRC PMIC */
> > + if (intel_dsi->gpio_panel)
> > + gpiod_set_value_cansleep(intel_dsi->gpio_panel, 0);
> > }
> >
> > static bool intel_dsi_get_hw_state(struct intel_encoder *encoder,
> > @@ -955,6 +964,11 @@ static void intel_dsi_encoder_destroy(struct
> > drm_encoder *encoder)
> > /* XXX: Logically this call belongs in the panel driver. */
> > drm_panel_remove(intel_dsi->panel);
> > }
> > +
> > + /* dispose of the gpios */
> > + if (intel_dsi->gpio_panel)
> > + gpiod_put(intel_dsi->gpio_panel);
> > +
> > intel_encoder_destroy(encoder);
> > }
> >
> > @@ -1071,6 +1085,20 @@ void intel_dsi_init(struct drm_device *dev)
> > goto err;
> > }
> >
> > + /*
> > +* In case of BYT with CRC PMIC, we need to use GPIO for
> > +* Panel control.
> > +*/
> > + if (dev_priv->vbt.dsi.config->pwm_blc == PPS_BLC_PMIC) {
> > + intel_dsi->gpio_panel =
> > + gpiod_get(dev->dev, "panel", GPIOD_OUT_HIGH);
> > +
> > + if (IS_ERR(intel_dsi->gpio_panel)) {
> > + DRM_ERROR("Failed to own gpio for panel control\n");
> > + intel_dsi->gpio_panel = NULL;
> > + }
> > + }
> > +
> > intel_encoder->type = INTEL_OUTPUT_DSI;
> > intel_encoder->cloneable = 0;
> > drm_connector_init(dev, connector, &intel_dsi_connector_funcs,
> > diff --git a/drivers/gpu/drm/i915/intel_dsi.h
> > b/drivers/gpu/drm/i915/intel_dsi.h
> > index 2784ac4..bf1bade 100644
> > --- a/drivers/gpu/drm/i915/intel_dsi.h
> > +++ b/drivers/gpu/drm/i915/intel_dsi.h
> > @@ -29,6 +29,9 @@
> > #include
> > #include "intel_drv.h"
> >
> > +#define PPS_BLC_PMIC 0
> > +#define PPS_BLC_SOC1
>
> Since these values are defined in the VBT, perhaps these should be in
> intel_bios.h. Up to you, I'm fine either way.
>
> Reviewed-by: Jani Nikula
Yeah if they're vbt it's better to keep all that together. Maybe we'll
eventually get a bright future where vbt documentation is solid, and then
it's good to have it all in one place.
-Daniel
>
>
> > +
> > /* Dual Link support */
> > #define DSI_DUAL_LINK_NONE 0
> > #define DSI_DUAL_LINK_FRONT_BACK 1
> > @@ -42,6 +45,9 @@ struct intel_dsi {
> > struct drm_panel *panel;
> > struct intel_dsi_host *dsi_hosts[I915_MAX_PORTS];
> >
> > + /* GPIO Desc for CRC based Panel control */
> > + struct gpio_desc *gpio_panel;
> > +
> > struct intel_connector *attached_connector;
> >
> > /* bit mask of ports being driven */
> > --
> > 2.1.0
> >
>
> --
> Jani Nikula, Intel Open Source Technology Center
> ___
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vett
[PATCH 5/5] drm/exynos: mixer: also allow ARGB1555 and ARGB4444
Allow the remaining alpha formats now that blending
is properly setup.
Signed-off-by: Tobias Jakobi
---
drivers/gpu/drm/exynos/exynos_mixer.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c
b/drivers/gpu/drm/exynos/exynos_mixer.c
index 430d10c..54e79e0 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -721,10 +721,12 @@ static void mixer_graph_buffer(struct mixer_context *ctx,
unsigned int win)
switch (plane->pixel_format) {
case DRM_FORMAT_XRGB:
+ case DRM_FORMAT_ARGB:
fmt = MXR_FORMAT_ARGB;
break;
case DRM_FORMAT_XRGB1555:
+ case DRM_FORMAT_ARGB1555:
fmt = MXR_FORMAT_ARGB1555;
break;
--
2.0.5
[PATCH 4/5] drm/exynos: mixer: do blending setup in mixer_cfg_layer()
This updates the blending setup when the layer configuration
changes (triggered by mixer_win_{commit,disable}).
Extra care has to be taken for the layer that is currently
being enabled/disabled.
Signed-off-by: Tobias Jakobi
---
drivers/gpu/drm/exynos/exynos_mixer.c | 23 +++
1 file changed, 23 insertions(+)
diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c
b/drivers/gpu/drm/exynos/exynos_mixer.c
index 3ab7a01..430d10c 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -177,6 +177,18 @@ static inline bool is_alpha_format(const struct
mixer_context* ctx, unsigned int
}
}
+static inline unsigned int layer_bitmask(const struct mixer_context* ctx)
+{
+ unsigned int i, mask = 0;
+
+ for (i = 0; i < MIXER_WIN_NR; ++i) {
+ if (ctx->planes[i].enabled)
+ mask |= (1 << i);
+ }
+
+ return mask;
+}
+
static inline u32 vp_reg_read(struct mixer_resources *res, u32 reg_id)
{
return readl(res->vp_regs + reg_id);
@@ -513,6 +525,7 @@ static void mixer_cfg_rgb_fmt(struct mixer_context *ctx,
unsigned int height)
static void mixer_cfg_layer(struct mixer_context *ctx, unsigned int win, bool
enable)
{
struct mixer_resources *res = &ctx->mixer_res;
+ unsigned int enable_state;
u32 val = enable ? ~0 : 0;
switch (win) {
@@ -530,6 +543,16 @@ static void mixer_cfg_layer(struct mixer_context *ctx,
unsigned int win, bool en
}
break;
}
+
+ /* Determine the current enabled/disabled state of the layers. */
+ enable_state = layer_bitmask(ctx);
+ if (enable)
+ enable_state |= (1 << win);
+ else
+ enable_state &= ~(1 << win);
+
+ /* Layer configuration has changed, update blending setup. */
+ mixer_layer_blending(ctx, enable_state);
}
static void mixer_run(struct mixer_context *ctx)
--
2.0.5
[PATCH 3/5] drm/exynos: mixer: remove all static blending setup
Previously blending setup was static and most of it was
done in mixer_win_reset().
Signed-off-by: Tobias Jakobi
---
drivers/gpu/drm/exynos/exynos_mixer.c | 23 ---
1 file changed, 23 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c
b/drivers/gpu/drm/exynos/exynos_mixer.c
index 5e95ef2..3ab7a01 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -527,11 +527,6 @@ static void mixer_cfg_layer(struct mixer_context *ctx,
unsigned int win, bool en
vp_reg_writemask(res, VP_ENABLE, val, VP_ENABLE_ON);
mixer_reg_writemask(res, MXR_CFG, val,
MXR_CFG_VP_ENABLE);
-
- /* control blending of graphic layer 0 */
- mixer_reg_writemask(res, MXR_GRAPHIC_CFG(0), val,
- MXR_GRP_CFG_BLEND_PRE_MUL |
- MXR_GRP_CFG_PIXEL_BLEND_EN);
}
break;
}
@@ -817,7 +812,6 @@ static void mixer_win_reset(struct mixer_context *ctx)
{
struct mixer_resources *res = &ctx->mixer_res;
unsigned long flags;
- u32 val; /* value stored to register */
spin_lock_irqsave(&res->reg_slock, flags);
mixer_vsync_set_update(ctx, false);
@@ -838,23 +832,6 @@ static void mixer_win_reset(struct mixer_context *ctx)
mixer_reg_write(res, MXR_BG_COLOR1, 0x008080);
mixer_reg_write(res, MXR_BG_COLOR2, 0x008080);
- /* setting graphical layers */
- val = MXR_GRP_CFG_COLOR_KEY_DISABLE; /* no blank key */
- val |= MXR_GRP_CFG_WIN_BLEND_EN;
- val |= MXR_GRP_CFG_ALPHA_VAL(0xff); /* non-transparent alpha */
-
- /* Don't blend layer 0 onto the mixer background */
- mixer_reg_write(res, MXR_GRAPHIC_CFG(0), val);
-
- /* Blend layer 1 into layer 0 */
- val |= MXR_GRP_CFG_BLEND_PRE_MUL;
- val |= MXR_GRP_CFG_PIXEL_BLEND_EN;
- mixer_reg_write(res, MXR_GRAPHIC_CFG(1), val);
-
- /* setting video layers */
- val = MXR_GRP_CFG_ALPHA_VAL(0);
- mixer_reg_write(res, MXR_VIDEO_CFG, val);
-
if (ctx->vp_enabled) {
/* configuration of Video Processor Registers */
vp_win_reset(ctx);
--
2.0.5
[PATCH 2/5] drm/exynos: mixer: introduce mixer_layer_blending()
This analyses the current layer configuration (which layers
are enabled, which have alpha-pixelformat, etc.) and setups
blending accordingly.
We currently disable all kinds of blending for the bottom-most
layer, since configuration of the mixer background is not
yet exposed.
Also blending is only enabled when the layer has a pixelformat
with alpha attached.
Signed-off-by: Tobias Jakobi
---
drivers/gpu/drm/exynos/exynos_mixer.c | 108 ++
drivers/gpu/drm/exynos/regs-mixer.h | 1 +
2 files changed, 109 insertions(+)
diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c
b/drivers/gpu/drm/exynos/exynos_mixer.c
index e4a5e76..5e95ef2 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -165,6 +165,18 @@ static const u8 filter_cr_horiz_tap4[] = {
70, 59, 48, 37, 27, 19, 11, 5,
};
+static inline bool is_alpha_format(const struct mixer_context* ctx, unsigned
int win)
+{
+ switch (ctx->planes[win].pixel_format) {
+ case DRM_FORMAT_ARGB:
+ case DRM_FORMAT_ARGB1555:
+ case DRM_FORMAT_ARGB:
+ return true;
+ default:
+ return false;
+ }
+}
+
static inline u32 vp_reg_read(struct mixer_resources *res, u32 reg_id)
{
return readl(res->vp_regs + reg_id);
@@ -322,6 +334,102 @@ static void mixer_layer_priority(struct mixer_context
*ctx)
mixer_reg_write(&ctx->mixer_res, MXR_LAYER_CFG, val);
}
+/* Configure blending for bottom-most layer. */
+static void mixer_bottom_layer(struct mixer_context *ctx,
+ const struct layer_config *cfg)
+{
+ u32 val;
+ struct mixer_resources *res = &ctx->mixer_res;
+
+ if (cfg->index == 2) {
+ val = 0; /* use defaults for video layer */
+ mixer_reg_write(res, MXR_VIDEO_CFG, val);
+ } else {
+ val = MXR_GRP_CFG_COLOR_KEY_DISABLE; /* no blank key */
+
+ /* Don't blend bottom-most layer onto the mixer background. */
+ mixer_reg_writemask(res, MXR_GRAPHIC_CFG(cfg->index),
+ val, MXR_GRP_CFG_MISC_MASK);
+ }
+}
+
+static void mixer_general_layer(struct mixer_context *ctx,
+ const struct layer_config *cfg)
+{
+ u32 val;
+ struct mixer_resources *res = &ctx->mixer_res;
+
+ if (is_alpha_format(ctx, cfg->index)) {
+ val = MXR_GRP_CFG_COLOR_KEY_DISABLE; /* no blank key */
+ val |= MXR_GRP_CFG_BLEND_PRE_MUL;
+ val |= MXR_GRP_CFG_PIXEL_BLEND_EN; /* blending based on pixel
alpha */
+
+ /* The video layer never has an alpha pixelformat. */
+ mixer_reg_writemask(res, MXR_GRAPHIC_CFG(cfg->index),
+ val, MXR_GRP_CFG_MISC_MASK);
+ } else {
+ if (cfg->index == 2) {
+ /*
+* No blending at the moment since the NV12/NV21
pixelformats don't
+* have an alpha channel. However the mixer supports a
global alpha
+* value for a layer. Once this functionality is
exposed, we can
+* support blending of the video layer through this.
+*/
+ val = 0;
+ mixer_reg_write(res, MXR_VIDEO_CFG, val);
+ } else {
+ val = MXR_GRP_CFG_COLOR_KEY_DISABLE;
+ mixer_reg_writemask(res, MXR_GRAPHIC_CFG(cfg->index),
+ val, MXR_GRP_CFG_MISC_MASK);
+ }
+ }
+}
+
+static void mixer_layer_blending(struct mixer_context *ctx, unsigned int
enable_state)
+{
+ const struct layer_config *cfg;
+ unsigned int i = 0;
+ unsigned int index;
+
+ /* Find bottom-most enabled layer. */
+ cfg = NULL;
+ while (i < ctx->num_layer) {
+ index = ctx->layer_config[i].index;
+ ++i;
+
+ if (enable_state & (1 << index)) {
+ cfg = &ctx->layer_config[i-1];
+ break;
+ }
+ }
+
+ /* No enabled layers found, nothing to do. */
+ if (!cfg)
+ return;
+
+ mixer_bottom_layer(ctx, cfg);
+
+ while (1) {
+ /* Find the next layer. */
+ cfg = NULL;
+ while (i < ctx->num_layer) {
+ index = ctx->layer_config[i].index;
+ ++i;
+
+ if (enable_state & (1 << index)) {
+ cfg = &ctx->layer_config[i-1];
+ break;
+ }
+ }
+
+ /* No more enabled layers found. */
+ if (!cfg)
+ return;
+
+ mixer_general_layer(ctx, cfg);
+ }
+}
+
static void mixer_vsync_set_update(s
[PATCH 1/5] drm/exynos: mixer: refactor layer setup
First step in allowing a more generic way to setup complex
blending for the different layers.
Signed-off-by: Tobias Jakobi
---
drivers/gpu/drm/exynos/exynos_mixer.c | 90 ++-
1 file changed, 79 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c
b/drivers/gpu/drm/exynos/exynos_mixer.c
index 4a1656b..e4a5e76 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -63,6 +63,11 @@ struct mixer_resources {
struct clk *mout_mixer;
};
+struct layer_config {
+ unsigned int index;
+ unsigned int priority;
+};
+
enum mixer_version_id {
MXR_VER_0_0_0_16,
MXR_VER_16_0_33_0,
@@ -75,6 +80,8 @@ struct mixer_context {
struct drm_device *drm_dev;
struct exynos_drm_crtc *crtc;
struct exynos_drm_plane planes[MIXER_WIN_NR];
+ const struct layer_config *layer_config;
+ unsigned int num_layer;
int pipe;
boolinterlace;
boolpowered;
@@ -95,6 +102,40 @@ struct mixer_drv_data {
boolhas_sclk;
};
+/*
+ * The default layer priorities. A higher priority means that
+ * the layer is at the top of layer stack.
+ * The current configuration assumes the following usage scenario:
+ * layer1: OSD [top]
+ * layer0: main framebuffer
+ * video layer: video overlay [bottom]
+ * Note that the video layer is only usable when the
+ * video processor is available.
+ */
+
+static const struct layer_config default_layer_config[] = {
+ {
+ .index = 0, /* layer0 */
+ .priority = 1
+ }, {
+ .index = 1, /* layer1 */
+ .priority = 2
+ }
+};
+
+static const struct layer_config vp_layer_config[] = {
+ {
+ .index = 2, /* video layer */
+ .priority = 1
+ }, {
+ .index = 0, /* layer0 */
+ .priority = 2
+ }, {
+ .index = 1, /* layer1 */
+ .priority = 3
+ }
+};
+
static const u8 filter_y_horiz_tap8[] = {
0, -1, -1, -1, -1, -1, -1, -1,
-1, -1, -1, -1, -1, 0, 0, 0,
@@ -253,6 +294,34 @@ static void vp_default_filter(struct mixer_resources *res)
filter_cr_horiz_tap4, sizeof(filter_cr_horiz_tap4));
}
+static void mixer_layer_priority(struct mixer_context *ctx)
+{
+ u32 val = 0;
+ unsigned int i, priority;
+
+ for (i = 0; i < ctx->num_layer; ++i) {
+ priority = ctx->layer_config[i].priority;
+ BUG_ON(priority > 15);
+
+ switch (ctx->layer_config[i].index) {
+ case 0:
+ val |= MXR_LAYER_CFG_GRP0_VAL(priority);
+ break;
+ case 1:
+ val |= MXR_LAYER_CFG_GRP1_VAL(priority);
+ break;
+ case 2:
+ val |= MXR_LAYER_CFG_VP_VAL(priority);
+ break;
+ default:
+ BUG_ON(true);
+ break;
+ }
+ }
+
+ mixer_reg_write(&ctx->mixer_res, MXR_LAYER_CFG, val);
+}
+
static void mixer_vsync_set_update(struct mixer_context *ctx, bool enable)
{
struct mixer_resources *res = &ctx->mixer_res;
@@ -654,17 +723,7 @@ static void mixer_win_reset(struct mixer_context *ctx)
mixer_reg_writemask(res, MXR_STATUS, MXR_STATUS_16_BURST,
MXR_STATUS_BURST_MASK);
- /* setting default layer priority: layer1 > layer0 > video
-* because typical usage scenario would be
-* layer1 - OSD
-* layer0 - framebuffer
-* video - video overlay
-*/
- val = MXR_LAYER_CFG_GRP1_VAL(3);
- val |= MXR_LAYER_CFG_GRP0_VAL(2);
- if (ctx->vp_enabled)
- val |= MXR_LAYER_CFG_VP_VAL(1);
- mixer_reg_write(res, MXR_LAYER_CFG, val);
+ mixer_layer_priority(ctx);
/* setting background color */
mixer_reg_write(res, MXR_BG_COLOR0, 0x008080);
@@ -1274,6 +1333,15 @@ static int mixer_probe(struct platform_device *pdev)
ctx->vp_enabled = drv->is_vp_enabled;
ctx->has_sclk = drv->has_sclk;
ctx->mxr_ver = drv->version;
+
+ if (drv->is_vp_enabled) {
+ ctx->layer_config = vp_layer_config;
+ ctx->num_layer = ARRAY_SIZE(vp_layer_config);
+ } else {
+ ctx->layer_config = default_layer_config;
+ ctx->num_layer = ARRAY_SIZE(default_layer_config);
+ }
+
init_waitqueue_head(&ctx->wait_vsync_queue);
atomic_set(&ctx->wait_vsync_event, 0);
--
2.0.5
[PATCH 0/5] drm/exynos: rework layer blending setup
Hello, this is a rework of the layer blending setup in the Exynos DRM mixer. The current setup is static and spread out through the mixer code. This rework pushes all the configuration details into a layer_config array, which specifies the priority of each layer. Two arrays are currently found in the code, one for SoC versions with a video processor (VP) and one for SoC versions without VP. The VP gives us one additional layer, the video layer, which natively supports the NV12/NV21 pixelformat. The blending setup roughly works like this: 1) Find the bottom-most enabled layer. Disable all blending for this layer. This is done because we currently don't expose modification of the mixer background to userspace. Once this is done we can add more flexibility here. 2) Find the next enabled layer in our layer stack. If the layer has a framebuffer with an alpha-pixelformat attached, enable blending for this layer. If not, disable blending. 3) Iterate (2) until all enabled layers are processed. The series has been tested on a Hardkernel Odroid-X2 (Exynos4412, which has a VP). If you want to use libdrm's modetest to check the series, please apply patches [1] and [2]. This should make it possible to also test a plane with NV12 format (which is located 'behind' the primary plane). With best wishes, Tobias [1] https://patchwork.kernel.org/patch/6349241/ [2] https://patchwork.kernel.org/patch/6349261/ Tobias Jakobi (5): drm/exynos: mixer: refactor layer setup drm/exynos: mixer: introduce mixer_layer_blending() drm/exynos: mixer: remove all static blending setup drm/exynos: mixer: do blending setup in mixer_cfg_layer() drm/exynos: mixer: also allow ARGB1555 and ARGB drivers/gpu/drm/exynos/exynos_mixer.c | 246 +- drivers/gpu/drm/exynos/regs-mixer.h | 1 + 2 files changed, 213 insertions(+), 34 deletions(-) -- 2.0.5
[RFC] How implement Secure Data Path ?
On Wed, May 06, 2015 at 11:19:21AM +0200, Thierry Reding wrote:
> On Wed, May 06, 2015 at 10:35:52AM +0200, Daniel Vetter wrote:
> > On Tue, May 05, 2015 at 05:54:05PM +0100, One Thousand Gnomes wrote:
> > > > First what is Secure Data Path ? SDP is a set of hardware features to
> > > > garanty
> > > > that some memories regions could only be read and/or write by specific
> > > > hardware
> > > > IPs. You can imagine it as a kind of memory firewall which grant/revoke
> > > > accesses to memory per devices. Firewall configuration must be done in
> > > > a trusted
> > > > environment: for ARM architecture we plan to use OP-TEE + a trusted
> > > > application to do that.
> > >
> > > It's not just an ARM feature so any basis for this in the core code
> > > should be generic, whether its being enforced by ARM SDP, various Intel
> > > feature sets or even via a hypervisor.
> > >
> > > > I have try 2 "hacky" approachs with dma_buf:
> > > > - add a secure field in dma_buf structure and configure firewall in
> > > > dma_buf_{map/unmap}_attachment() functions.
> > >
> > > How is SDP not just another IOMMU. The only oddity here is that it
> > > happens to configure buffers the CPU can't touch and it has a control
> > > mechanism that is designed to cover big media corp type uses where the
> > > threat model is that the system owner is the enemy. Why does anything care
> > > about it being SDP, there are also generic cases this might be a useful
> > > optimisation (eg knowing the buffer isn't CPU touched so you can optimise
> > > cache flushing).
> > >
> > > The control mechanism is a device/platform detail as with any IOMMU. It
> > > doesn't matter who configures it or how, providing it happens.
> > >
> > > We do presumably need some small core DMA changes - anyone trying to map
> > > such a buffer into CPU space needs to get a warning or error but what
> > > else ?
> > >
> > > > >From buffer allocation point of view I also facing a problem because
> > > > >when v4l2
> > > > or drm/kms are exporting buffers by using dma_buf they don't attaching
> > > > themself on it and never call dma_buf_{map/unmap}_attachment(). This is
> > > > not
> > > > an issue in those framework while it is how dma_buf exporters are
> > > > supposed to work.
> > >
> > > Which could be addressed if need be.
> > >
> > > So if "SDP" is just another IOMMU feature, just as stuff like IMR is on
> > > some x86 devices, and hypervisor enforced protection is on assorted
> > > platforms why do we need a special way to do it ? Is there anything
> > > actually needed beyond being able to tell the existing DMA code that this
> > > buffer won't be CPU touched and wiring it into the DMA operations for the
> > > platform ?
> >
> > Iirc most of the dma api stuff gets unhappy when memory isn't struct page
> > backed. In i915 we do use sg tables everywhere though (even for memory not
> > backed by struct page, e.g. the "stolen" range the bios prereserves), but
> > we fill those out manually.
> >
> > A possible generic design I see is to have a secure memory allocator
> > device which doesn nothing else but hand out dma-bufs.
>
> Are you suggesting a device file with a custom set of IOCTLs for this?
> Or some in-kernel API that would perform the secure allocations? I
> suspect the former would be better suited, because it gives applications
> the control over whether they need secure buffers or not. The latter
> would require custom extensions in every driver to make them allocate
> from a secure memory pool.
Yes the idea would be a special-purpose allocater thing like ion. Might
even want that to be a syscall to do it properly.
> For my understanding, would the secure memory allocator be responsible
> for setting up the permissions to access the memory at attachment time?
Well not permission checks, but hw capability checks. The allocator should
have platform knowledge about which devices can access such secure memory
(since some will definitely not be able to do that for fear of just plain
sending it out to the world).
> >With that we can
> > hide the platform-specific allocation methods in there (some need to
> > allocate from carveouts, other just need to mark the pages specifically).
> > Also dma-buf has explicit methods for cpu access, which are allowed to
> > fail. And using the dma-buf attach tracking we can also reject dma to
> > devices which cannot access the secure memory. Given all that I think
> > going through the dma-buf interface but with a special-purpose allocator
> > seems to fit.
>
> That sounds like a flexible enough design to me. I think it's something
> that we could easily implement on Tegra. The memory controller on Tegra
> implements this using a special video-protect aperture (VPR) and memory
> clients can individually be allowed access to this aperture. That means
> VPR is a carveout that is typically set up by some secure firmware, and
> that in turn, as
[Bug 90284] GPU lockup with DOTA2
https://bugs.freedesktop.org/show_bug.cgi?id=90284 --- Comment #5 from Trippler --- (In reply to Trippler from comment #4) > Created attachment 115595 [details] > journalctl -r -x --boot=-1 when recreating the crash with R600_DEBUG=ps,vs,gs Done. I added two logs - "output_steam.gz" which is the output of the program, and "journal_log_with_debug_ps_vs_gs.gz" which is the output of "journalctl --boot=-1 -r -x" -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150506/7f828315/attachment.html>
[Bug 90284] GPU lockup with DOTA2
https://bugs.freedesktop.org/show_bug.cgi?id=90284 --- Comment #4 from Trippler --- Created attachment 115595 --> https://bugs.freedesktop.org/attachment.cgi?id=115595&action=edit journalctl -r -x --boot=-1 when recreating the crash with R600_DEBUG=ps,vs,gs -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150506/557f3766/attachment.html>
[Bug 90284] GPU lockup with DOTA2
https://bugs.freedesktop.org/show_bug.cgi?id=90284 --- Comment #3 from Trippler --- Created attachment 115594 --> https://bugs.freedesktop.org/attachment.cgi?id=115594&action=edit Output of the program being run with R600_DEBUG=ps,vs,gs export R600_DEBUG=ps,vs,gs steam > output_steam After this I ran game from steam and recreated crash condition. Also attached (next attachment) is systemd log from this event as well (journal_log_with_debug_ps_vs_gs.gz) -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150506/0bc7b323/attachment.html>
[PATCH v3] libgencec: Add userspace library for the generic CEC kernel interface
Add the libGenCEC library. It was designed to act as an interface between the generic CEC kernel API and userspace applications. It provides a simple interface for applications and an example application that can be used to test the CEC functionality. Signed-off-by: Kamil Debski --- Hi, This is the third version of the patch adding a simple library for using with the Kernel's Generic CEC Interface. Changes since v2 I have added the changes suggested by Emil Velikov that did not make it in the v2. - added .gitignore files (and removed m4/.gitkeep) - added AC_PREREQ and LT_PREREQ - added autogen.sh script - fixed examples/Makefile.am (use *_LDADD instead of AM_LDFLAGS - added -no-undefined to src/Makefile.am Changes since v1 - added libgencec.pc.in file Best wishes, Kamil Debski --- .gitignore | 26 +++ AUTHORS |1 + INSTALL |8 + LICENSE | 202 Makefile.am |4 + README | 22 ++ autogen.sh | 13 ++ configure.ac | 27 +++ doc/index.html | 345 +++ examples/Makefile.am |4 + examples/cectest.c | 631 ++ include/gencec.h | 255 libgencec.pc.in | 10 + m4/.gitignore|5 + src/Makefile.am |4 + src/gencec.c | 445 +++ 16 files changed, 2002 insertions(+) create mode 100644 .gitignore create mode 100644 AUTHORS create mode 100644 INSTALL create mode 100644 LICENSE create mode 100644 Makefile.am create mode 100644 README create mode 100755 autogen.sh create mode 100644 configure.ac create mode 100644 doc/index.html create mode 100644 examples/Makefile.am create mode 100644 examples/cectest.c create mode 100644 include/gencec.h create mode 100644 libgencec.pc.in create mode 100644 m4/.gitignore create mode 100644 src/Makefile.am create mode 100644 src/gencec.c diff --git a/.gitignore b/.gitignore new file mode 100644 index 000..8ea993d --- /dev/null +++ b/.gitignore @@ -0,0 +1,26 @@ +*.la +*.lo +*.o + +/aclocal.m4 +/ar-lib +/autom4te.cache/* +/config.* +/configure +/depcomp +/examples/.deps/* +/examples/.libs/* +/examples/cectest +/install-sh + +libgencec.pc +libtool +ltmain.sh +Makefile +Makefile.in + +/missing +src/.deps/* +src/.libs/* + + diff --git a/AUTHORS b/AUTHORS new file mode 100644 index 000..e4b7117 --- /dev/null +++ b/AUTHORS @@ -0,0 +1 @@ +Kamil Debski diff --git a/INSTALL b/INSTALL new file mode 100644 index 000..ae60a1f --- /dev/null +++ b/INSTALL @@ -0,0 +1,8 @@ +To install libgencec run following commands: + +./autogen.sh +make +make install + +A cross compilation example for ARM: +CFLAGS=-I ./autogen.sh --host=arm-linux-gnueabi --prefix= diff --git a/LICENSE b/LICENSE new file mode 100644 index 000..d645695 --- /dev/null +++ b/LICENSE @@ -0,0 +1,202 @@ + + Apache License + Version 2.0, January 2004 +http://www.apache.org/licenses/ + + TERMS AND CONDITIONS FOR USE, REPRODUCTION, AND DISTRIBUTION + + 1. Definitions. + + "License" shall mean the terms and conditions for use, reproduction, + and distribution as defined by Sections 1 through 9 of this document. + + "Licensor" shall mean the copyright owner or entity authorized by + the copyright owner that is granting the License. + + "Legal Entity" shall mean the union of the acting entity and all + other entities that control, are controlled by, or are under common + control with that entity. For the purposes of this definition, + "control" means (i) the power, direct or indirect, to cause the + direction or management of such entity, whether by contract or + otherwise, or (ii) ownership of fifty percent (50%) or more of the + outstanding shares, or (iii) beneficial ownership of such entity. + + "You" (or "Your") shall mean an individual or Legal Entity + exercising permissions granted by this License. + + "Source" form shall mean the preferred form for making modifications, + including but not limited to software source code, documentation + source, and configuration files. + + "Object" form shall mean any form resulting from mechanical + transformation or translation of a Source form, including but + not limited to compiled object code, generated documentation, + and conversions to other media types. + + "Work" shall mean the work of authorship, whether in Source or + Object form, made available under the License, as indicated by a + copyright notice that is included in or attached to the work + (an example is provided in the Appendix below). + + "Derivative Works" shall mean any work, whether in Source or Object + form, that is based on (or derived from) the
[PATCH v2] modetest: only select plane with matching format
Don't assume that a plane supports any kind of pixelformat
but do a check first.
v2: Simplify the format check.
Signed-off-by: Tobias Jakobi
---
tests/modetest/modetest.c | 14 +-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
index 7f0c1cc..29d4c34 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -969,6 +969,18 @@ page_flip_handler(int fd, unsigned int frame,
}
}
+static bool format_support(const drmModePlanePtr ovr, uint32_t fmt)
+{
+ unsigned int i;
+
+ for (i = 0; i < ovr->count_formats; ++i) {
+ if (ovr->formats[i] == fmt)
+ return true;
+ }
+
+ return false;
+}
+
static int set_plane(struct device *dev, struct plane_arg *p)
{
drmModePlane *ovr;
@@ -999,7 +1011,7 @@ static int set_plane(struct device *dev, struct plane_arg
*p)
for (i = 0; i < dev->resources->plane_res->count_planes && !plane_id;
i++) {
ovr = dev->resources->planes[i].plane;
- if (!ovr)
+ if (!ovr || !format_support(ovr, p->fourcc))
continue;
if ((ovr->possible_crtcs & (1 << pipe)) && !ovr->crtc_id)
--
2.0.5
[PATCH] libgencec: Add userspace library for the generic CEC kernel interface
Hi Emil, From: Emil Velikov [mailto:emil.l.veli...@gmail.com] Sent: Tuesday, May 05, 2015 5:03 PM > > Hi Kamil, > > It seems that you've only incorporated the libgencec.pc suggestion. > Did you change your mind about the others, found out something funny > with them (if so can you let me know what) or simply forgot about them ? I have to admit that I was sending the patchset in a hurry and I forgot to merge all the changes in the lib. I am sorry. I have prepared an updated version just now and I am going to send it soon. > > On 30 April 2015 at 11:25, Kamil Debski wrote: > > Hi Emil, > > > > From: linux-media-owner at vger.kernel.org [mailto:linux-media- > > owner at vger.kernel.org] On Behalf Of Emil Velikov > > Sent: Wednesday, April 29, 2015 5:00 PM > >> > >> Hi Kamil, > >> > >> Allow me to put a few suggestions: > >> > >> On 29 April 2015 at 11:02, Kamil Debski wrote: > > >> > diff --git a/m4/.gitkeep b/m4/.gitkeep new file mode 100644 index > >> > 000..e69de29 > >> Haven't seen any projects doing this. Curious what the benefits of > >> keeping and empty folder might be ? > > > > When I run autoreconf -i it complained about missing m4 folder, hence > > I added this filler file such that the folder is created. Any > > suggestion on how to do this better? > > > Ahh yes - that lovely message. It turns out that older versions of > automake will even error out [1], rather than just printing out a > warning. A handy workaround would be to add a .gitignore (and a second > one in the top folder) list. Plus it will slim down the untracked files > list and let you clearly see when git add was missed :-) Adding .gitignore files is a good suggestion. Will do. > > Cheers > Emil > > [1] https://bugzilla.redhat.com/show_bug.cgi?id=901333 Best wishes, -- Kamil Debski Samsung R&D Institute Poland
[PATCH 2/2] modetest: only select plane with matching format
Don't assume that a plane supports any kind of pixelformat
but do a check first.
Signed-off-by: Tobias Jakobi
---
tests/modetest/modetest.c | 18 +-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
index 7f0c1cc..f5f00ba 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -969,6 +969,22 @@ page_flip_handler(int fd, unsigned int frame,
}
}
+static bool format_support(const drmModePlanePtr ovr, uint32_t fmt)
+{
+ unsigned int i;
+ bool ret = false;
+
+ for (i = 0; i < ovr->count_formats; ++i) {
+ if (ovr->formats[i] != fmt)
+ continue;
+
+ ret = true;
+ break;
+ }
+
+ return ret;
+}
+
static int set_plane(struct device *dev, struct plane_arg *p)
{
drmModePlane *ovr;
@@ -999,7 +1015,7 @@ static int set_plane(struct device *dev, struct plane_arg
*p)
for (i = 0; i < dev->resources->plane_res->count_planes && !plane_id;
i++) {
ovr = dev->resources->planes[i].plane;
- if (!ovr)
+ if (!ovr || !format_support(ovr, p->fourcc))
continue;
if ((ovr->possible_crtcs & (1 << pipe)) && !ovr->crtc_id)
--
2.0.5
[PATCH 1/2] modetest: make middle SMPTE colors transparent
This enables us to check for overlay planes which are located
'below' the primary plane.
Since the alpha value only has an effect when creating surfaces
with an alpha-pixelformat this doesn't affect the regular
XRGB primary surface.
Signed-off-by: Tobias Jakobi
---
tests/modetest/buffers.c | 28 ++--
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/tests/modetest/buffers.c b/tests/modetest/buffers.c
index 7e214e8..30ac033 100644
--- a/tests/modetest/buffers.c
+++ b/tests/modetest/buffers.c
@@ -423,13 +423,13 @@ fill_smpte_rgb16(const struct rgb_info *rgb, unsigned
char *mem,
MAKE_RGBA(rgb, 0, 0, 192, 255), /* blue */
};
const uint16_t colors_middle[] = {
- MAKE_RGBA(rgb, 0, 0, 192, 255), /* blue */
- MAKE_RGBA(rgb, 19, 19, 19, 255),/* black */
- MAKE_RGBA(rgb, 192, 0, 192, 255), /* magenta */
- MAKE_RGBA(rgb, 19, 19, 19, 255),/* black */
- MAKE_RGBA(rgb, 0, 192, 192, 255), /* cyan */
- MAKE_RGBA(rgb, 19, 19, 19, 255),/* black */
- MAKE_RGBA(rgb, 192, 192, 192, 255), /* grey */
+ MAKE_RGBA(rgb, 0, 0, 192, 127), /* blue */
+ MAKE_RGBA(rgb, 19, 19, 19, 127),/* black */
+ MAKE_RGBA(rgb, 192, 0, 192, 127), /* magenta */
+ MAKE_RGBA(rgb, 19, 19, 19, 127),/* black */
+ MAKE_RGBA(rgb, 0, 192, 192, 127), /* cyan */
+ MAKE_RGBA(rgb, 19, 19, 19, 127),/* black */
+ MAKE_RGBA(rgb, 192, 192, 192, 127), /* grey */
};
const uint16_t colors_bottom[] = {
MAKE_RGBA(rgb, 0, 33, 76, 255), /* in-phase */
@@ -547,13 +547,13 @@ fill_smpte_rgb32(const struct rgb_info *rgb, unsigned
char *mem,
MAKE_RGBA(rgb, 0, 0, 192, 255), /* blue */
};
const uint32_t colors_middle[] = {
- MAKE_RGBA(rgb, 0, 0, 192, 255), /* blue */
- MAKE_RGBA(rgb, 19, 19, 19, 255),/* black */
- MAKE_RGBA(rgb, 192, 0, 192, 255), /* magenta */
- MAKE_RGBA(rgb, 19, 19, 19, 255),/* black */
- MAKE_RGBA(rgb, 0, 192, 192, 255), /* cyan */
- MAKE_RGBA(rgb, 19, 19, 19, 255),/* black */
- MAKE_RGBA(rgb, 192, 192, 192, 255), /* grey */
+ MAKE_RGBA(rgb, 0, 0, 192, 127), /* blue */
+ MAKE_RGBA(rgb, 19, 19, 19, 127),/* black */
+ MAKE_RGBA(rgb, 192, 0, 192, 127), /* magenta */
+ MAKE_RGBA(rgb, 19, 19, 19, 127),/* black */
+ MAKE_RGBA(rgb, 0, 192, 192, 127), /* cyan */
+ MAKE_RGBA(rgb, 19, 19, 19, 127),/* black */
+ MAKE_RGBA(rgb, 192, 192, 192, 127), /* grey */
};
const uint32_t colors_bottom[] = {
MAKE_RGBA(rgb, 0, 33, 76, 255), /* in-phase */
--
2.0.5
[PATCH 2/8] pwm: core: Add support to remove registered consumer lookup tables
On Tue, May 05, 2015 at 03:04:18PM +0530, Shobhit Kumar wrote: > In case some drivers are unloading, they can remove lookup tables which > they would have registered during their load time to avoid redundant > entries if loaded again > > v2: Ccing maintainers > v3: Correct the subject line (Lee jones) > > CC: Samuel Ortiz > Cc: Linus Walleij > Cc: Alexandre Courbot > Cc: Thierry Reding > Signed-off-by: Shobhit Kumar > --- > drivers/pwm/core.c | 17 + > include/linux/pwm.h | 5 + > 2 files changed, 22 insertions(+) Applied, thanks. Thierry -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150506/ad641c3c/attachment.sig>
[Bug 90284] GPU lockup with DOTA2
https://bugs.freedesktop.org/show_bug.cgi?id=90284 --- Comment #2 from Tom Stellard --- Can you run the program with the environment variable R600_DEBUG=ps,vs,gs and post the output. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150506/34d8853f/attachment-0001.html>
[PATCH 6/8] pwm: crc: Add Crystalcove (CRC) PWM driver
ice *pwm,
> + int duty_ns, int period_ns)
Please align arguments on subsequent lines with the first argument of
the first line.
> +{
> + struct crystalcove_pwm *crc_pwm = to_crc_pwm(c);
> + struct device *dev = &crc_pwm->pdev->dev;
Did you test reconfiguring the PWM? I don't see crc_pwm->pdev getting
initialized anywhere, so this should crash trying to dereference a NULL
pointer.
Of course if you get rid of the pdev field as I suggested you can simply
get the struct device * from c->dev.
> + int level, percent;
> +
> + if (period_ns > PWM_MAX_PERIOD_NS) {
> + dev_err(dev, "un-supported period_ns\n");
> + return -1;
You should return -EINVAL here. Besides being a literal and therefore a
bad idea, -1 == -EPERM and doesn't match the error condition.
> + }
> +
> + if (pwm->period != period_ns) {
> + int clk_div;
> +
> + /* changing the clk divisor, need to disable fisrt */
> + crc_pwm_disable(c, pwm);
> + clk_div = PWM_BASE_CLK * period_ns / 100;
Similar to the above, this is confusing because you're mixing up
different scales here. period_ns is in nanoseconds, so it'd be natural
to divide by 10 (though you should really be using NSEC_PER_SEC
instead). If you counterweight that by expressing PWM_BASE_CLK in Hz
(600) you get much nicer symmetry and make the code a lot easier to
understand.
> +
> + regmap_write(crc_pwm->regmap, PWM0_CLK_DIV,
> + clk_div | PWM_OUTPUT_ENABLE);
> +
> + /* enable back */
> + crc_pwm_enable(c, pwm);
> + }
> +
> + if (duty_ns > period_ns) {
> + dev_err(dev, "duty cycle cannot be greater than cycle
> period\n");
> + return -1;
> + }
The PWM core already performs this check, so you'll never get here in
case this condition is true.
> +
> + /* change the pwm duty cycle */
> + percent = duty_ns * 100 / period_ns;
> + level = percent * PWM_MAX_LEVEL / 100;
Why do you need to apply the rule of three twice here? Doesn't
level = duty_ns * PWM_MAX_LEVEL / period_ns;
give you what you want?
> + regmap_write(crc_pwm->regmap, PWM0_DUTY_CYCLE, level);
> +
> + return 0;
> +}
> +
> +static const struct pwm_ops crc_pwm_ops = {
> + .config = crc_pwm_config,
> + .enable = crc_pwm_enable,
> + .disable = crc_pwm_disable,
> + .owner = THIS_MODULE,
> +};
> +
> +static int crystalcove_pwm_probe(struct platform_device *pdev)
> +{
> + struct crystalcove_pwm *pwm;
> + int retval;
> + struct device *dev = pdev->dev.parent;
> + struct intel_soc_pmic *pmic = dev_get_drvdata(dev);
> +
> + pwm = devm_kzalloc(&pdev->dev, sizeof(*pwm), GFP_KERNEL);
> + if (!pwm)
> + return -ENOMEM;
> +
> + pwm->chip.dev = &pdev->dev;
> + pwm->chip.ops = &crc_pwm_ops;
> + pwm->chip.base = -1;
> + pwm->chip.npwm = 1;
> +
> + /* get the PMIC regmap */
> + pwm->regmap = pmic->regmap;
> +
> + retval = pwmchip_add(&pwm->chip);
> + if (retval < 0)
> + return retval;
> +
> + dev_dbg(&pdev->dev, "crc-pwm probe successful\n");
Do you really want this? The driver core will complain in any of the
above failures, so what use is there to be chatty when probing
succeeds?
> +static struct platform_driver crystalcove_pwm_driver = {
> + .probe = crystalcove_pwm_probe,
> + .remove = crystalcove_pwm_remove,
> + .driver = {
> + .name = "crystal_cove_pwm",
I'd prefer this to be "crystal-cove-pwm" for consistency with other
drivers, but since the MFD part already uses underscores in names it'd
introduce an inconsistency there. So I'm fine with this one as-is.
Thierry
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20150506/3f350163/attachment.sig>
[PATCH 2/2] drm/exynos: 'win' is always unsigned
The index for the hardware layer is always >=0. Previous
code that also used -1 as special index is now gone.
Also apply this to 'ch_enabled' (decon/fimd), since the
variable is on the same line (and is again always unsigned).
Signed-off-by: Tobias Jakobi
---
drivers/gpu/drm/exynos/exynos7_drm_decon.c | 2 +-
drivers/gpu/drm/exynos/exynos_drm_fimd.c | 6 +++---
drivers/gpu/drm/exynos/exynos_mixer.c | 6 +++---
3 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos7_drm_decon.c
b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
index 1f7e33f..5c0b2cc 100644
--- a/drivers/gpu/drm/exynos/exynos7_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
@@ -91,7 +91,7 @@ static void decon_wait_for_vblank(struct exynos_drm_crtc
*crtc)
static void decon_clear_channel(struct decon_context *ctx)
{
- int win, ch_enabled = 0;
+ unsigned int win, ch_enabled = 0;
DRM_DEBUG_KMS("%s\n", __FILE__);
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index 9819fa6..ee11ea6 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -216,7 +216,7 @@ static void fimd_wait_for_vblank(struct exynos_drm_crtc
*crtc)
DRM_DEBUG_KMS("vblank wait timed out.\n");
}
-static void fimd_enable_video_output(struct fimd_context *ctx, int win,
+static void fimd_enable_video_output(struct fimd_context *ctx, unsigned int
win,
bool enable)
{
u32 val = readl(ctx->regs + WINCON(win));
@@ -229,7 +229,7 @@ static void fimd_enable_video_output(struct fimd_context
*ctx, int win,
writel(val, ctx->regs + WINCON(win));
}
-static void fimd_enable_shadow_channel_path(struct fimd_context *ctx, int win,
+static void fimd_enable_shadow_channel_path(struct fimd_context *ctx, unsigned
int win,
bool enable)
{
u32 val = readl(ctx->regs + SHADOWCON);
@@ -244,7 +244,7 @@ static void fimd_enable_shadow_channel_path(struct
fimd_context *ctx, int win,
static void fimd_clear_channel(struct fimd_context *ctx)
{
- int win, ch_enabled = 0;
+ unsigned int win, ch_enabled = 0;
DRM_DEBUG_KMS("%s\n", __FILE__);
diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c
b/drivers/gpu/drm/exynos/exynos_mixer.c
index ff15702..4a1656b 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -333,7 +333,7 @@ static void mixer_cfg_rgb_fmt(struct mixer_context *ctx,
unsigned int height)
mixer_reg_writemask(res, MXR_CFG, val, MXR_CFG_RGB_FMT_MASK);
}
-static void mixer_cfg_layer(struct mixer_context *ctx, int win, bool enable)
+static void mixer_cfg_layer(struct mixer_context *ctx, unsigned int win, bool
enable)
{
struct mixer_resources *res = &ctx->mixer_res;
u32 val = enable ? ~0 : 0;
@@ -379,7 +379,7 @@ static void mixer_stop(struct mixer_context *ctx)
usleep_range(1, 12000);
}
-static void vp_video_buffer(struct mixer_context *ctx, int win)
+static void vp_video_buffer(struct mixer_context *ctx, unsigned int win)
{
struct mixer_resources *res = &ctx->mixer_res;
unsigned long flags;
@@ -511,7 +511,7 @@ fail:
return -ENOTSUPP;
}
-static void mixer_graph_buffer(struct mixer_context *ctx, int win)
+static void mixer_graph_buffer(struct mixer_context *ctx, unsigned int win)
{
struct mixer_resources *res = &ctx->mixer_res;
unsigned long flags;
--
2.0.5
[PATCH 1/2] drm/exynos: mixer: don't dump registers under spinlock
mixer_regs_dump() was called in mixer_run(), which was called under the register spinlock in mixer_graph_buffer() and vp_video_buffer(). This would trigger a sysmmu pagefault with drm.debug=0xff because of the large delay caused by the register dumping. To keep consistency also move register dumping out of mixer_stop(), which is the counterpart to mixer_run(). Kernel dump: [ 131.296529] [drm:mixer_win_commit] win: 2 [ 131.300693] [drm:mixer_regs_dump] MXR_STATUS = 0081 [ 131.305888] [drm:mixer_regs_dump] MXR_CFG = 07d5 [ 131.310835] [drm:mixer_regs_dump] MXR_INT_EN = [ 131.316043] [drm:mixer_regs_dump] MXR_INT_STATUS = 0900 [ 131.321598] [drm:mixer_regs_dump] MXR_LAYER_CFG = 0321 [ 131.327066] [drm:mixer_regs_dump] MXR_VIDEO_CFG = [ 131.332535] [drm:mixer_regs_dump] MXR_GRAPHIC0_CFG = 00310700 [ 131.338263] [drm:mixer_regs_dump] MXR_GRAPHIC0_BASE = 20c0 [ 131.344079] [drm:mixer_regs_dump] MXR_GRAPHIC0_SPAN = 0780 [ 131.349895] [drm:mixer_regs_dump] MXR_GRAPHIC0_WH = 07800438 [ 131.355537] [drm:mixer_regs_dump] MXR_GRAPHIC0_SXY = [ 131.361265] [drm:mixer_regs_dump] MXR_GRAPHIC0_DXY = [ 131.366994] [drm:mixer_regs_dump] MXR_GRAPHIC1_CFG = [ 131.372723] [drm:mixer_regs_dump] MXR_GRAPHIC1_BASE = [ 131.378539] [drm:mixer_regs_dump] MXR_GRAPHIC1_SPAN = [ 131.384354] [drm:mixer_regs_dump] MXR_GRAPHIC1_WH = [ 131.389996] [drm:mixer_regs_dump] MXR_GRAPHIC1_SXY = [ 131.395725] [drm:mixer_regs_dump] MXR_GRAPHIC1_DXY = [ 131.401486] PAGE FAULT occurred at 0x0 by 12e2.sysmmu(Page table base: 0x6d99) [ 131.409353] Lv1 entry: 0x6e0f2401 [ 131.412753] [ cut here ] [ 131.417339] kernel BUG at drivers/iommu/exynos-iommu.c:358! [ 131.422894] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP ARM [ 131.428709] Modules linked in: ecb bridge stp llc bnep btrfs xor xor_neon zlib_inflate zlib_deflate raid6_pq btusb bluetooth usb_storage s5p_jpeg videobuf2_dma_contig videobuf2_memops v4l2_mem2mem videobuf2_core [ 131.447461] CPU: 0 PID: 2418 Comm: lt-modetest Tainted: GW 4.0.1-debug+ #3 [ 131.455530] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree) [ 131.461607] task: ee194100 ti: ec4fe000 task.ti: ec4fe000 [ 131.466995] PC is at exynos_sysmmu_irq+0x2a0/0x2a8 [ 131.471766] LR is at vprintk_emit+0x268/0x594 [ 131.476103] pc : []lr : []psr: a1d3 [ 131.476103] sp : ec4ff9d8 ip : fp : ec4ffa14 [ 131.487559] r10: ffda r9 : ee206e28 r8 : ee2d1a10 [ 131.492767] r7 : r6 : r5 : r4 : ee206e10 [ 131.499277] r3 : c06fca20 r2 : r1 : r0 : ee28be00 [ 131.505788] Flags: NzCv IRQs off FIQs off Mode SVC_32 ISA ARM Segment user [ 131.513079] Control: 10c5387d Table: 6c72404a DAC: 0015 [ 131.518808] Process lt-modetest (pid: 2418, stack limit = 0xec4fe218) [ 131.525231] Stack: (0xec4ff9d8 to 0xec50) [ 131.529571] f9c0: ec4ff9e4 c03a0c40 [ 131.537732] f9e0: bbfa6e35 6d99 6d161c3d ee20a900 ee04a7e0 0028 ee007000 [ 131.545891] fa00: c06fb1fc ec4ffa5c ec4ffa18 c0066a34 c0277f10 ee257664 000b [ 131.554050] fa20: ec4ffa5c c06fafbb ee04a780 c06fb1e8 ee04a780 ee04a7e0 ee20a900 [ 131.562209] fa40: ee007000 0015 ec4ffb48 ee008000 ec4ffa7c ec4ffa60 c0066c90 c00669e0 [ 131.570369] fa60: 0002 ee04a780 ee04a7e0 1000 ec4ffa94 ec4ffa80 c0069c6c c0066c58 [ 131.578528] fa80: 0028 ee004450 ec4ffaac ec4ffa98 c0066028 c0069bac 00a0 c06e19b4 [ 131.586687] faa0: ec4ffad4 ec4ffab0 c0223678 c0066000 c02235dc 0015 0015 [ 131.594846] fac0: ec4ffc80 0001 ec4ffaec ec4ffad8 c0066028 c02235e8 0089 c06bfc54 [ 131.603005] fae0: ec4ffb1c ec4ffaf0 c006633c c0066000 ec4ffb48 f002000c 0025 0015 [ 131.611165] fb00: c06c680c ec4ffb48 f002 ee008000 ec4ffb44 ec4ffb20 c000867c c00662c4 [ 131.619324] fb20: c02046ac 6153 ec4ffb7c 0101 ec4ffbb4 ec4ffb48 [ 131.627483] fb40: c0013240 c0008650 0001 ee257508 0002 0001 ee257504 ee257508 [ 131.635642] fb60: c06bf27c 0101 ee008000 ec4ffbb4 ec4ffb90 [ 131.643802] fb80: c002e124 c02046ac 6153 c002e09c c06c6080 0283 [ 131.651960] fba0: 0001 c06fb1ac ec4ffc0c ec4ffbb8 c002d690 c002e0a8 ee78d080 ee008000 [ 131.660120] fbc0: 0040 c04eb3b0 7c44 c06c6100 c06fdac0 000a c06bf2f0 c06c6080 [ 131.668279] fbe0: c06bfc54 c06bfc54 0025 0001 ec4ffc80 ee008000 [ 131.676438] fc00: ec4ffc24 ec4ffc10 c002dbb8 c002d564 0089 c06bfc54 ec4ffc54 ec4ffc28 [ 131.684597] fc20: c0066340 c002dafc ec4ffc80 f002000c 001c 000c c06c680c ec4ffc80 [ 131.692757] fc40: f002 0080 ec4ffc7c ec4ffc58 c000867c c00662c4 c04e6624 6053 [ 131.700916] fc6
[Bug 90340] RADEON "PITCAIRN" displayport output breaks when monitor turned off and on
https://bugs.freedesktop.org/show_bug.cgi?id=90340 Bug ID: 90340 Summary: RADEON "PITCAIRN" displayport output breaks when monitor turned off and on Product: DRI Version: XOrg git Hardware: x86-64 (AMD64) OS: Linux (All) Status: NEW Severity: normal Priority: medium Component: DRM/Radeon Assignee: dri-devel at lists.freedesktop.org Reporter: steve at einval.com As initially reporte in Debian at https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=783705 I'm running Debian Jessie (3.16.x kernel) on a machine with a Pitcairn-based video adapter and an NEC Multisync EA274WMi 27" monitor, connected via DisplayPort. I've also tried upgrading to the 4.0 kernel and I see exactly the same symptoms. If I turn the monitor off and then on again (as I was doing overnight), I get no display at all. I've wiggled the mouse, hit numlock on the keybard (the numlock led illuminates fine), etc., but no display. I've seen this kind of thing happen in the past on some machines, so I switch to VT1 and back to see if that helps. Still no display at all, either on console or under X. I log in remotely and I can see that the Xorg.0.log file has been updated with mode lines for the monitor, suggesting things have just woken up fine. But still no display. Here's the really weird thing: at this point, the monitor has basically locked up. It won't respond to the power/input/menu butttons at all, and is still showing the blue LED that says "I have signal" rather than switching to the amber "no signal" warning. Therefore, I can only assume there's a problem here with some weird invalid DP signal being produced. I've carried on playing with this. If I log in from another machine and run xrandr, I can see the display details of the monitor just fine. If I use xrandr to turn the display off and then on again: xrandr --output DisplayPort-0 --off xrandr --output DisplayPort-0 2560x1440 then things start working again. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150506/53d5d25c/attachment-0001.html>
[i915]] *ERROR* mismatch in scaler_state.scaler_id
Adding Chandra, who's implemented skl scaler code. -Daniel On Sat, May 02, 2015 at 10:05:42AM +0900, Sergey Senozhatsky wrote: > Hi, > > linux-next 20150501 > > [1.968953] [drm:check_crtc_state [i915]] *ERROR* mismatch in > scaler_state.scaler_id (expected 0, found -1) > [1.968953] [ cut here ] > [1.968983] WARNING: CPU: 0 PID: 6 at > drivers/gpu/drm/i915/intel_display.c:12008 check_crtc_state+0xb15/0xb83 > [i915]() > [1.968983] pipe state doesn't match! > [..] > [1.969005] CPU: 0 PID: 6 Comm: kworker/u16:0 Not tainted > 4.1.0-rc1-next-20150501-dbg-00011-gbcb7bed-dirty #49 > [1.969010] Workqueue: events_unbound async_run_entry_fn > [1.969012] 0009 88041d9eb448 812e9753 > > [1.969013] 88041d9eb498 88041d9eb488 81034c24 > 88041d9eb490 > [1.969015] a03a81dc 88041d123000 88041cbbc800 > 0001 > [1.969015] Call Trace: > [1.969019] [] dump_stack+0x45/0x57 > [1.969022] [] warn_slowpath_common+0x97/0xb1 > [1.969050] [] ? check_crtc_state+0xb15/0xb83 [i915] > [1.969052] [] warn_slowpath_fmt+0x41/0x43 > [1.969080] [] check_crtc_state+0xb15/0xb83 [i915] > [1.969082] [] ? update_curr+0x68/0xd1 > [1.969112] [] intel_modeset_check_state+0x603/0xa3d > [i915] > [1.969140] [] intel_crtc_set_config+0x8dc/0xc02 [i915] > [1.969147] [] ? > drm_atomic_helper_plane_set_property+0x6c/0xa4 [drm_kms_helper] > [1.969159] [] drm_mode_set_config_internal+0x57/0xe3 > [drm] > [1.969164] [] restore_fbdev_mode+0xb5/0xcf > [drm_kms_helper] > [1.969169] [] > drm_fb_helper_restore_fbdev_mode_unlocked+0x22/0x59 [drm_kms_helper] > [1.969173] [] drm_fb_helper_set_par+0x31/0x35 > [drm_kms_helper] > [1.969202] [] intel_fbdev_set_par+0x15/0x58 [i915] > [1.969204] [] fbcon_init+0x323/0x431 > [1.969206] [] visual_init+0xb7/0x10d > [1.969208] [] do_bind_con_driver+0x1b1/0x2d8 > [1.969209] [] do_take_over_console+0x15a/0x184 > [1.969212] [] do_fbcon_takeover+0x5b/0x97 > [1.969213] [] fbcon_event_notify+0x419/0x740 > [1.969215] [] notifier_call_chain+0x3b/0x5f > [1.969217] [] __blocking_notifier_call_chain+0x43/0x5f > [1.969219] [] blocking_notifier_call_chain+0xf/0x11 > [1.969220] [] fb_notifier_call_chain+0x16/0x18 > [1.969222] [] register_framebuffer+0x28f/0x2c7 > [1.969250] [] ? intelfb_create+0x2e7/0x38a [i915] > [1.969256] [] drm_fb_helper_initial_config+0x297/0x3e1 > [drm_kms_helper] > [1.969284] [] intel_fbdev_initial_config+0x16/0x18 > [i915] > [1.969286] [] async_run_entry_fn+0x33/0xca > [1.969288] [] process_one_work+0x192/0x2a8 > [1.969290] [] worker_thread+0x266/0x34c > [1.969291] [] ? rescuer_thread+0x276/0x276 > [1.969293] [] kthread+0xcd/0xd5 > [1.969294] [] ? kthread_worker_fn+0x130/0x130 > [1.969296] [] ret_from_fork+0x42/0x70 > [1.969297] [] ? kthread_worker_fn+0x130/0x130 > [1.969298] ---[ end trace 7f37d8e5ab4ee0a8 ]--- > > -ss > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[Bug 15006] translate & rotate the line cause Aliasing
https://bugs.freedesktop.org/show_bug.cgi?id=15006 marius predut changed: What|Removed |Added Assignee|nanhai.zou at intel.com|marius.predut at intel.com --- Comment #5 from marius predut --- The patch series that fix this bug are upstream -- You are receiving this mail because: You are on the CC list for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150506/148a6a37/attachment.html>
[PATCH 9/9] drm/exynos: Convert g2d_userptr_get_dma_addr() to use get_vaddr_frames()
On 05/06/2015 09:28 AM, Jan Kara wrote:
> Convert g2d_userptr_get_dma_addr() to pin pages using get_vaddr_frames().
> This removes the knowledge about vmas and mmap_sem locking from exynos
> driver. Also it fixes a problem that the function has been mapping user
> provided address without holding mmap_sem.
>
> Signed-off-by: Jan Kara
> ---
> drivers/gpu/drm/exynos/exynos_drm_g2d.c | 89 ++
> drivers/gpu/drm/exynos/exynos_drm_gem.c | 97
> -
> 2 files changed, 29 insertions(+), 157 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_g2d.c
> b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
> index 81a250830808..265519c0fe2d 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_g2d.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
...
> @@ -456,65 +458,37 @@ static dma_addr_t *g2d_userptr_get_dma_addr(struct
> drm_device *drm_dev,
> return ERR_PTR(-ENOMEM);
>
> atomic_set(&g2d_userptr->refcount, 1);
> + g2d_userptr->size = size;
>
> start = userptr & PAGE_MASK;
> offset = userptr & ~PAGE_MASK;
> end = PAGE_ALIGN(userptr + size);
> npages = (end - start) >> PAGE_SHIFT;
> - g2d_userptr->npages = npages;
> -
> - pages = drm_calloc_large(npages, sizeof(struct page *));
> - if (!pages) {
> - DRM_ERROR("failed to allocate pages.\n");
> - ret = -ENOMEM;
> + vec = g2d_userptr->vec = frame_vector_create(npages);
> + if (!vec)
> goto err_free;
> - }
>
> - down_read(¤t->mm->mmap_sem);
> - vma = find_vma(current->mm, userptr);
> - if (!vma) {
> - up_read(¤t->mm->mmap_sem);
> - DRM_ERROR("failed to get vm region.\n");
> + ret = get_vaddr_frames(start, npages, 1, 1, vec);
Use true instead of 1.
[PATCH 3/9] media: omap_vout: Convert omap_vout_uservirt_to_phys() to use get_vaddr_pfns()
On 05/06/2015 09:28 AM, Jan Kara wrote:
> Convert omap_vout_uservirt_to_phys() to use get_vaddr_pfns() instead of
> hand made mapping of virtual address to physical address. Also the
> function leaked page reference from get_user_pages() so fix that by
> properly release the reference when omap_vout_buffer_release() is
> called.
>
> Signed-off-by: Jan Kara
> ---
> drivers/media/platform/omap/omap_vout.c | 67
> +++--
> 1 file changed, 31 insertions(+), 36 deletions(-)
>
...
> + vec = frame_vector_create(1);
> + if (!vec)
> + return -ENOMEM;
>
> - if (res == nr_pages) {
> - physp = __pa(page_address(&pages[0]) +
> - (virtp & ~PAGE_MASK));
> - } else {
> - printk(KERN_WARNING VOUT_NAME
> - "get_user_pages failed\n");
> - return 0;
> - }
> + ret = get_vaddr_frames(virtp, 1, 1, 0, vec);
Use true/false where appropriate.
[PATCH 2/9] mm: Provide new get_vaddr_frames() helper
On 05/06/2015 09:28 AM, Jan Kara wrote:
> Provide new function get_vaddr_frames(). This function maps virtual
> addresses from given start and fills given array with page frame numbers of
> the corresponding pages. If given start belongs to a normal vma, the function
> grabs reference to each of the pages to pin them in memory. If start
> belongs to VM_IO | VM_PFNMAP vma, we don't touch page structures. Caller
> must make sure pfns aren't reused for anything else while he is using
> them.
>
> This function is created for various drivers to simplify handling of
> their buffers.
>
> Signed-off-by: Jan Kara
Acked-by: Vlastimil Babka
With some nitpicks below...
> ---
> include/linux/mm.h | 44 +++
> mm/gup.c | 214
> +
> 2 files changed, 258 insertions(+)
>
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 0755b9fd03a7..dcd1f02a78e9 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -20,6 +20,7 @@
> #include
> #include
> #include
> +#include
>
> struct mempolicy;
> struct anon_vma;
> @@ -1197,6 +1198,49 @@ long get_user_pages_unlocked(struct task_struct *tsk,
> struct mm_struct *mm,
> int write, int force, struct page **pages);
> int get_user_pages_fast(unsigned long start, int nr_pages, int write,
> struct page **pages);
> +
> +/* Container for pinned pfns / pages */
> +struct frame_vector {
> + unsigned int nr_allocated; /* Number of frames we have space for */
> + unsigned int nr_frames; /* Number of frames stored in ptrs array */
> + bool got_ref; /* Did we pin pages by getting page ref? */
> + bool is_pfns; /* Does array contain pages or pfns? */
> + void *ptrs[0]; /* Array of pinned pfns / pages. Use
> + * pfns_vector_pages() or pfns_vector_pfns()
> + * for access */
> +};
> +
> +struct frame_vector *frame_vector_create(unsigned int nr_frames);
> +void frame_vector_destroy(struct frame_vector *vec);
> +int get_vaddr_frames(unsigned long start, unsigned int nr_pfns,
> + bool write, bool force, struct frame_vector *vec);
> +void put_vaddr_frames(struct frame_vector *vec);
> +int frame_vector_to_pages(struct frame_vector *vec);
> +void frame_vector_to_pfns(struct frame_vector *vec);
> +
> +static inline unsigned int frame_vector_count(struct frame_vector *vec)
> +{
> + return vec->nr_frames;
> +}
> +
> +static inline struct page **frame_vector_pages(struct frame_vector *vec)
> +{
> + if (vec->is_pfns) {
> + int err = frame_vector_to_pages(vec);
> +
> + if (err)
> + return ERR_PTR(err);
> + }
> + return (struct page **)(vec->ptrs);
> +}
> +
> +static inline unsigned long *frame_vector_pfns(struct frame_vector *vec)
> +{
> + if (!vec->is_pfns)
> + frame_vector_to_pfns(vec);
> + return (unsigned long *)(vec->ptrs);
> +}
> +
> struct kvec;
> int get_kernel_pages(const struct kvec *iov, int nr_pages, int write,
> struct page **pages);
> diff --git a/mm/gup.c b/mm/gup.c
> index 6297f6bccfb1..8db5c40e65c4 100644
> --- a/mm/gup.c
> +++ b/mm/gup.c
> @@ -8,6 +8,7 @@
> #include
> #include
> #include
> +#include
>
> #include
> #include
> @@ -936,6 +937,219 @@ int __mm_populate(unsigned long start, unsigned long
> len, int ignore_errors)
> return ret; /* 0 or negative error code */
> }
>
> +/*
> + * get_vaddr_frames() - map virtual addresses to pfns
> + * @start: starting user address
> + * @nr_frames: number of pages / pfns from start to map
> + * @write: whether pages will be written to by the caller
> + * @force: whether to force write access even if user mapping is
> + * readonly. This will result in the page being COWed even
> + * in MAP_SHARED mappings. You do not want this.
"You do not want this" and yet some of the drivers in later patches do.
That looks weird. Explain better?
> + * @vec: structure which receives pages / pfns of the addresses mapped.
> + * It should have space for at least nr_frames entries.
> + *
> + * This function maps virtual addresses from @start and fills @vec structure
> + * with page frame numbers or page pointers to corresponding pages (choice
> + * depends on the type of the vma underlying the virtual address). If @start
> + * belongs to a normal vma, the function grabs reference to each of the pages
> + * to pin them in memory. If @start belongs to VM_IO | VM_PFNMAP vma, we
> don't
> + * touch page structures and the caller must make sure pfns aren't reused for
> + * anything else while he is using them.
> + *
> + * The function returns number of pages mapped which may be less than
> + * @nr_frames. In particular we stop mapping if there are more vmas of
> + * different type underlying the specified range of virt
component_unbind_all() splat..
On Wed, May 6, 2015 at 12:01 PM, Russell King - ARM Linux
wrote:
> On Wed, May 06, 2015 at 10:22:22AM -0400, Rob Clark wrote:
>> Hey Russell,
>>
>> I just noticed a splat triggered by component_unbind_all() called from
>> ->bind().. any opinions about whether component stuff should handle
>> that case, or whether I should rearrange my error cleanup path to not
>> component_unbind_all() in this case?
>
> Essentially, if component_bind_all() returned an error, you should not
> call component_unbind_all() - component_bind_all() has the effect that
> it's either successful and all components have been bound, or when it
> fails, no components are bound.
>
> component_unbind_all() assumes that the components were previously
> successfully bound.
>
> When I look at the MSM code, I can see why this happens, and it's
> basically because of broken error cleanup handling caused by this
> commit:
Right, what to do to fix it in msm is clear enough.. what I was
(trying to) get at was, since error paths are perpetually undertested,
would it make more sense to handle component_unbind_all() called from
->bind().. (although I also didn't go audit the other component users
yet to see if any might have the same issue)
BR,
-R
> commit 5bf9c0b614542d69fb9a8681a0411715cc3e8ba8
> Author: Rob Clark
> Date: Tue Mar 3 15:04:24 2015 -0500
>
> drm/msm: split out vram initialization
>
> We'll want to extend this a bit to handle also a reserved-memory
> ("stolen") region, so that drm/msm can take-over bootloader splash
> screen. First split it out into it's own fxn to reduce noise in
> the following patch.
>
> Signed-off-by: Rob Clark
>
> Let's look at the clean-up paths:
>
> priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> if (!priv) {
> dev_err(dev->dev, "failed to allocate private data\n");
> return -ENOMEM;
>
> returns immediately.
>
> ret = msm_init_vram(dev);
> if (ret)
> goto fail;
>
> calls msm_unload.
>
> /* Bind all our sub-components: */
> ret = component_bind_all(dev->dev, dev);
> if (ret)
> return ret;
>
> doesn't call msm_unload.
>
> Why would the second not need to run any cleanup if failing at
> msm_init_vram() does? This is totally mad.
>
> I think you need to fix this driver to have a sane error cleanup
> implementation...
>
> --
> FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
> according to speedtest.net.
[PATCH] drm/msm: Fix compil issue when DRM_MSM_FBDEV is disabled
> On Wed, May 6, 2015 at 9:25 AM, Stephane Viau
> wrote:
>> When CONFIG_DRM_MSM_FBDEV is not defined,
>> CONFIG_DRM_KMS_FB_HELPER does not get selected and
>> drm_fb_helper_*() helper functions are thus not available.
>>
>> This change fixes these link issues.
>
> Hmm, didn't Archit start on making fbdev config option global and
> adding nop-stubs for the case that it was disabled? I lost track of
> where that was going..
Oh right... https://lkml.org/lkml/2015/2/23/99
I'll take my patch back then ;-)
Thanks,
Stephane.
>
> BR,
> -R
>
>> Signed-off-by: Stephane Viau
>> ---
>> drivers/gpu/drm/msm/msm_drv.c | 4
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/msm/msm_drv.c
>> b/drivers/gpu/drm/msm/msm_drv.c
>> index 2b1218c..35380ec 100644
>> --- a/drivers/gpu/drm/msm/msm_drv.c
>> +++ b/drivers/gpu/drm/msm/msm_drv.c
>> @@ -21,9 +21,11 @@
>>
>> static void msm_fb_output_poll_changed(struct drm_device *dev)
>> {
>> +#ifdef DRM_MSM_FBDEV
>> struct msm_drm_private *priv = dev->dev_private;
>> if (priv->fbdev)
>> drm_fb_helper_hotplug_event(priv->fbdev);
>> +#endif
>> }
>>
>> static const struct drm_mode_config_funcs mode_config_funcs = {
>> @@ -419,9 +421,11 @@ static void msm_preclose(struct drm_device *dev,
>> struct drm_file *file)
>>
>> static void msm_lastclose(struct drm_device *dev)
>> {
>> +#ifdef DRM_MSM_FBDEV
>> struct msm_drm_private *priv = dev->dev_private;
>> if (priv->fbdev)
>> drm_fb_helper_restore_fbdev_mode_unlocked(priv->fbdev);
>> +#endif
>> }
>>
>> static irqreturn_t msm_irq(int irq, void *arg)
>> --
>> Qualcomm Innovation Center, Inc.
>>
>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
>> Forum, a Linux Foundation Collaborative Project
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm"
> in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
[RFC] How implement Secure Data Path ?
I agree that the best solution is to have a generic dmabuf allocator
but no only for secure use cases.
If we create a memory allocator dedicated to security it means that
userland will be responsible to use it or not depending of the context
which may change while the pipeline/graph is already running...
Renegotiate buffers allocation in "live" is very difficult and takes time.
To keep this simple to use a memory allocator device is probably the
best solution but Sumit have already to propose this kind of solution
with the "constraint aware" allocator without succes.
Does secure data path requirements will be enough to make this acceptable now ?
2015-05-06 10:35 GMT+02:00 Daniel Vetter :
> On Tue, May 05, 2015 at 05:54:05PM +0100, One Thousand Gnomes wrote:
>> > First what is Secure Data Path ? SDP is a set of hardware features to
>> > garanty
>> > that some memories regions could only be read and/or write by specific
>> > hardware
>> > IPs. You can imagine it as a kind of memory firewall which grant/revoke
>> > accesses to memory per devices. Firewall configuration must be done in a
>> > trusted
>> > environment: for ARM architecture we plan to use OP-TEE + a trusted
>> > application to do that.
>>
>> It's not just an ARM feature so any basis for this in the core code
>> should be generic, whether its being enforced by ARM SDP, various Intel
>> feature sets or even via a hypervisor.
>>
>> > I have try 2 "hacky" approachs with dma_buf:
>> > - add a secure field in dma_buf structure and configure firewall in
>> > dma_buf_{map/unmap}_attachment() functions.
>>
>> How is SDP not just another IOMMU. The only oddity here is that it
>> happens to configure buffers the CPU can't touch and it has a control
>> mechanism that is designed to cover big media corp type uses where the
>> threat model is that the system owner is the enemy. Why does anything care
>> about it being SDP, there are also generic cases this might be a useful
>> optimisation (eg knowing the buffer isn't CPU touched so you can optimise
>> cache flushing).
>>
>> The control mechanism is a device/platform detail as with any IOMMU. It
>> doesn't matter who configures it or how, providing it happens.
>>
>> We do presumably need some small core DMA changes - anyone trying to map
>> such a buffer into CPU space needs to get a warning or error but what
>> else ?
>>
>> > >From buffer allocation point of view I also facing a problem because when
>> > >v4l2
>> > or drm/kms are exporting buffers by using dma_buf they don't attaching
>> > themself on it and never call dma_buf_{map/unmap}_attachment(). This is not
>> > an issue in those framework while it is how dma_buf exporters are
>> > supposed to work.
>>
>> Which could be addressed if need be.
>>
>> So if "SDP" is just another IOMMU feature, just as stuff like IMR is on
>> some x86 devices, and hypervisor enforced protection is on assorted
>> platforms why do we need a special way to do it ? Is there anything
>> actually needed beyond being able to tell the existing DMA code that this
>> buffer won't be CPU touched and wiring it into the DMA operations for the
>> platform ?
>
> Iirc most of the dma api stuff gets unhappy when memory isn't struct page
> backed. In i915 we do use sg tables everywhere though (even for memory not
> backed by struct page, e.g. the "stolen" range the bios prereserves), but
> we fill those out manually.
>
> A possible generic design I see is to have a secure memory allocator
> device which doesn nothing else but hand out dma-bufs. With that we can
> hide the platform-specific allocation methods in there (some need to
> allocate from carveouts, other just need to mark the pages specifically).
> Also dma-buf has explicit methods for cpu access, which are allowed to
> fail. And using the dma-buf attach tracking we can also reject dma to
> devices which cannot access the secure memory. Given all that I think
> going through the dma-buf interface but with a special-purpose allocator
> seems to fit.
>
> I'm not sure whether a special iommu is a good idea otoh: I'd expect that
> for most devices the driver would need to decide about which iommu to pick
> (or maybe keep track of some special flags for an extended dma_map
> interface). At least looking at gpu drivers using iommus would require
> special code, whereas fully hiding all this behind the dma-buf interface
> should fit in much better.
> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
--
Benjamin Gaignard
Graphic Working Group
Linaro.org â Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
[RFC] How implement Secure Data Path ?
at it should work out fine to not require struct page backed memory for this infrastructure since by definition the CPU won't be allowed to access it anyway. > I'm not sure whether a special iommu is a good idea otoh: I'd expect that > for most devices the driver would need to decide about which iommu to pick > (or maybe keep track of some special flags for an extended dma_map > interface). At least looking at gpu drivers using iommus would require > special code, whereas fully hiding all this behind the dma-buf interface > should fit in much better. As I understand it, even though the VPR on Tegra is a carveout it still is subject to IOMMU translation. So if IOMMU translation is enabled for a device (say the display controller), then all accesses to memory will be translated, whether they are to VPR or non-protected memory. Again I think this should work out fine with a special secure allocator. If the SG tables are filled in properly drivers should be able to cope. It's possible that existing IOMMU drivers would require modification to make this work, though. For example, the default_iommu_map_sg() function currently uses sg_page(), so that wouldn't be able to map secure buffers to I/O virtual addresses. That said, drivers could reimplement this on top of iommu_map(), though that may imply suboptimal performance on the mapping operation. Similarly some backing implementations of the DMA API rely on struct page pointers being present. But it seems more like you wouldn't want to use the DMA API at all if you want to use this kind of protected memory. Thierry -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150506/cccafcfe/attachment-0001.sig>
[Intel-gfx] [PATCH] drm/vblank: Fixup and document timestamp update/read barriers
On Tue, May 05, 2015 at 11:57:42AM -0400, Peter Hurley wrote:
> On 05/05/2015 11:42 AM, Daniel Vetter wrote:
> > On Tue, May 05, 2015 at 10:36:24AM -0400, Peter Hurley wrote:
> >> On 05/04/2015 12:52 AM, Mario Kleiner wrote:
> >>> On 04/16/2015 03:03 PM, Daniel Vetter wrote:
> On Thu, Apr 16, 2015 at 08:30:55AM -0400, Peter Hurley wrote:
> > On 04/15/2015 01:31 PM, Daniel Vetter wrote:
> >> On Wed, Apr 15, 2015 at 09:00:04AM -0400, Peter Hurley wrote:
> >>> Hi Daniel,
> >>>
> >>> On 04/15/2015 03:17 AM, Daniel Vetter wrote:
> This was a bit too much cargo-culted, so lets make it solid:
> - vblank->count doesn't need to be an atomic, writes are always done
> under the protection of dev->vblank_time_lock. Switch to an
> unsigned
> long instead and update comments. Note that atomic_read is just a
> normal read of a volatile variable, so no need to audit all the
> read-side access specifically.
>
> - The barriers for the vblank counter seqlock weren't complete: The
> read-side was missing the first barrier between the counter read
> and
> the timestamp read, it only had a barrier between the ts and the
> counter read. We need both.
>
> - Barriers weren't properly documented. Since barriers only work if
> you have them on boths sides of the transaction it's prudent to
> reference where the other side is. To avoid duplicating the
> write-side comment 3 times extract a little store_vblank() helper.
> In that helper also assert that we do indeed hold
> dev->vblank_time_lock, since in some cases the lock is acquired a
> few functions up in the callchain.
>
> Spotted while reviewing a patch from Chris Wilson to add a fastpath
> to
> the vblank_wait ioctl.
>
> Cc: Chris Wilson
> Cc: Mario Kleiner
> Cc: Ville Syrjälä
> Cc: Michel Dänzer
> Signed-off-by: Daniel Vetter
> ---
> drivers/gpu/drm/drm_irq.c | 92
> ---
> include/drm/drmP.h| 8 +++--
> 2 files changed, 54 insertions(+), 46 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> index c8a34476570a..23bfbc61a494 100644
> --- a/drivers/gpu/drm/drm_irq.c
> +++ b/drivers/gpu/drm/drm_irq.c
> @@ -74,6 +74,33 @@ module_param_named(vblankoffdelay,
> drm_vblank_offdelay, int, 0600);
> module_param_named(timestamp_precision_usec,
> drm_timestamp_precision, int, 0600);
> module_param_named(timestamp_monotonic, drm_timestamp_monotonic,
> int, 0600);
>
> +static void store_vblank(struct drm_device *dev, int crtc,
> + unsigned vblank_count_inc,
> + struct timeval *t_vblank)
> +{
> +struct drm_vblank_crtc *vblank = &dev->vblank[crtc];
> +u32 tslot;
> +
> +assert_spin_locked(&dev->vblank_time_lock);
> +
> +if (t_vblank) {
> +tslot = vblank->count + vblank_count_inc;
> +vblanktimestamp(dev, crtc, tslot) = *t_vblank;
> +}
> +
> +/*
> + * vblank timestamp updates are protected on the write side with
> + * vblank_time_lock, but on the read side done locklessly using
> a
> + * sequence-lock on the vblank counter. Ensure correct ordering
> using
> + * memory barrriers. We need the barrier both before and also
> after the
> + * counter update to synchronize with the next timestamp write.
> + * The read-side barriers for this are in
> drm_vblank_count_and_time.
> + */
> +smp_wmb();
> +vblank->count += vblank_count_inc;
> +smp_wmb();
> >>>
> >>> The comment and the code are each self-contradictory.
> >>>
> >>> If vblank->count writes are always protected by vblank_time_lock
> >>> (something I
> >>> did not verify but that the comment above asserts), then the trailing
> >>> write
> >>> barrier is not required (and the assertion that it is in the comment
> >>> is incorrect).
> >>>
> >>> A spin unlock operation is always a write barrier.
> >>
> >> Hm yeah. Otoh to me that's bordering on "code too clever for my own
> >> good".
> >> That the spinlock is held I can assure. That no one goes around and
> >> does
> >> multiple vblank updates (because somehow that code raced with the hw
> >> itself) I can't easily assure with a simple assert or something
> >> si
[RFC] How implement Secure Data Path ?
On 05/06/15 10:35, Daniel Vetter wrote:
> On Tue, May 05, 2015 at 05:54:05PM +0100, One Thousand Gnomes wrote:
>>> First what is Secure Data Path ? SDP is a set of hardware features to
>>> garanty
>>> that some memories regions could only be read and/or write by specific
>>> hardware
>>> IPs. You can imagine it as a kind of memory firewall which grant/revoke
>>> accesses to memory per devices. Firewall configuration must be done in a
>>> trusted
>>> environment: for ARM architecture we plan to use OP-TEE + a trusted
>>> application to do that.
>>
>> It's not just an ARM feature so any basis for this in the core code
>> should be generic, whether its being enforced by ARM SDP, various Intel
>> feature sets or even via a hypervisor.
>>
>>> I have try 2 "hacky" approachs with dma_buf:
>>> - add a secure field in dma_buf structure and configure firewall in
>>> dma_buf_{map/unmap}_attachment() functions.
>>
>> How is SDP not just another IOMMU. The only oddity here is that it
>> happens to configure buffers the CPU can't touch and it has a control
>> mechanism that is designed to cover big media corp type uses where the
>> threat model is that the system owner is the enemy. Why does anything care
>> about it being SDP, there are also generic cases this might be a useful
>> optimisation (eg knowing the buffer isn't CPU touched so you can optimise
>> cache flushing).
>>
>> The control mechanism is a device/platform detail as with any IOMMU. It
>> doesn't matter who configures it or how, providing it happens.
>>
>> We do presumably need some small core DMA changes - anyone trying to map
>> such a buffer into CPU space needs to get a warning or error but what
>> else ?
>>
>>> >From buffer allocation point of view I also facing a problem because when
>>> >v4l2
>>> or drm/kms are exporting buffers by using dma_buf they don't attaching
>>> themself on it and never call dma_buf_{map/unmap}_attachment(). This is not
>>> an issue in those framework while it is how dma_buf exporters are
>>> supposed to work.
>>
>> Which could be addressed if need be.
>>
>> So if "SDP" is just another IOMMU feature, just as stuff like IMR is on
>> some x86 devices, and hypervisor enforced protection is on assorted
>> platforms why do we need a special way to do it ? Is there anything
>> actually needed beyond being able to tell the existing DMA code that this
>> buffer won't be CPU touched and wiring it into the DMA operations for the
>> platform ?
>
> Iirc most of the dma api stuff gets unhappy when memory isn't struct page
> backed. In i915 we do use sg tables everywhere though (even for memory not
> backed by struct page, e.g. the "stolen" range the bios prereserves), but
> we fill those out manually.
>
> A possible generic design I see is to have a secure memory allocator
> device which doesn nothing else but hand out dma-bufs. With that we can
> hide the platform-specific allocation methods in there (some need to
> allocate from carveouts, other just need to mark the pages specifically).
> Also dma-buf has explicit methods for cpu access, which are allowed to
> fail.
BTW, v4l2 currently doesn't use those cpu access calls. It should, though,
and I have patches for that. However, I haven't had time to clean them up
and post them. I remember that I had problems with one or two drivers as
well, but I can't remember if I solved those problems or not.
I would expect that in order to implement SDP you need to get the cpu
access part sorted as well, so this should help.
My latest tree for this work is here:
http://git.linuxtv.org/cgit.cgi/hverkuil/media_tree.git/log/?h=vb2-prep5
I tried to rebase but that is a bit more involved than I have time for
right now. If someone really wants this then let me know and I can rebase
it for you.
> And using the dma-buf attach tracking we can also reject dma to
> devices which cannot access the secure memory. Given all that I think
> going through the dma-buf interface but with a special-purpose allocator
> seems to fit.
I agree with that. I think we discussed this when dma-buf was designed
that it should be possible to be used to represent opaque memory.
Regards,
Hans
> I'm not sure whether a special iommu is a good idea otoh: I'd expect that
> for most devices the driver would need to decide about which iommu to pick
> (or maybe keep track of some special flags for an extended dma_map
> interface). At least looking at gpu drivers using iommus would require
> special code, whereas fully hiding all this behind the dma-buf interface
> should fit in much better.
> -Daniel
>
[RFC] How implement Secure Data Path ?
2015-05-05 18:54 GMT+02:00 One Thousand Gnomes :
>> First what is Secure Data Path ? SDP is a set of hardware features to garanty
>> that some memories regions could only be read and/or write by specific
>> hardware
>> IPs. You can imagine it as a kind of memory firewall which grant/revoke
>> accesses to memory per devices. Firewall configuration must be done in a
>> trusted
>> environment: for ARM architecture we plan to use OP-TEE + a trusted
>> application to do that.
>
> It's not just an ARM feature so any basis for this in the core code
> should be generic, whether its being enforced by ARM SDP, various Intel
> feature sets or even via a hypervisor.
I agree the core code should be generic, I was just mention OP-TEE to explain on
which context I'm working.
>
>> I have try 2 "hacky" approachs with dma_buf:
>> - add a secure field in dma_buf structure and configure firewall in
>> dma_buf_{map/unmap}_attachment() functions.
>
> How is SDP not just another IOMMU. The only oddity here is that it
> happens to configure buffers the CPU can't touch and it has a control
> mechanism that is designed to cover big media corp type uses where the
> threat model is that the system owner is the enemy. Why does anything care
> about it being SDP, there are also generic cases this might be a useful
> optimisation (eg knowing the buffer isn't CPU touched so you can optimise
> cache flushing).
>
IOMMU interface doesn't offer API to manage buffer refcounting so you
will ignore
when you will have to stop protect the memory when using dma_buf you know
that the buffer is release when destroy function is called.
It is not only to not allow CPU to access to memory but also to
configure hardware
devices and firewall to be able to access to the memory.
> The control mechanism is a device/platform detail as with any IOMMU. It
> doesn't matter who configures it or how, providing it happens.
>
> We do presumably need some small core DMA changes - anyone trying to map
> such a buffer into CPU space needs to get a warning or error but what
> else ?
>
>> >From buffer allocation point of view I also facing a problem because when
>> >v4l2
>> or drm/kms are exporting buffers by using dma_buf they don't attaching
>> themself on it and never call dma_buf_{map/unmap}_attachment(). This is not
>> an issue in those framework while it is how dma_buf exporters are
>> supposed to work.
>
> Which could be addressed if need be.
>
> So if "SDP" is just another IOMMU feature, just as stuff like IMR is on
> some x86 devices, and hypervisor enforced protection is on assorted
> platforms why do we need a special way to do it ? Is there anything
> actually needed beyond being able to tell the existing DMA code that this
> buffer won't be CPU touched and wiring it into the DMA operations for the
> platform ?
>
> Alan
--
Benjamin Gaignard
Graphic Working Group
Linaro.org â Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
[RFC] How implement Secure Data Path ?
On Wed, May 06, 2015 at 03:50:13AM +0300, Laurent Pinchart wrote: > On Tuesday 05 May 2015 09:27:52 Christoph Hellwig wrote: > > On Tue, May 05, 2015 at 05:39:57PM +0200, Benjamin Gaignard wrote: > > > Since few months I'm looking for Linaro to how do Secure Data Path (SPD). > > > I have tried and implemented multiple thinks but I always facing > > > architecture issues so I would like to get your help to solve the > > > problem. > > > > > > First what is Secure Data Path ? SDP is a set of hardware features to > > > garanty that some memories regions could only be read and/or write by > > > specific hardware IPs. You can imagine it as a kind of memory firewall > > > which grant/revoke accesses to memory per devices. Firewall configuration > > > must be done in a trusted environment: for ARM architecture we plan to > > > use OP-TEE + a trusted application to do that. > > > > > > One typical use case for SDP in a video playback which involve those > > > elements: decrypt -> video decoder -> transform -> display > > > > Sounds like a good enough reason not to implement it ever. > > The irony of it is to post an RFC on they day before > http://www.defectivebydesign.org/dayagainstdrm/ :-) Just for the record: Even though I disagree with the design&threat model for secure memory I don't think we should outright refuse to merge patches. Assuming it comes with a sane design and no blob bits I'd be very much willing to merge support for i915. Unfortunately Intel isn't willing to publish the specs for any of the content protection stuff, at least right now. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[RFC] How implement Secure Data Path ?
On Tue, May 05, 2015 at 05:54:05PM +0100, One Thousand Gnomes wrote:
> > First what is Secure Data Path ? SDP is a set of hardware features to
> > garanty
> > that some memories regions could only be read and/or write by specific
> > hardware
> > IPs. You can imagine it as a kind of memory firewall which grant/revoke
> > accesses to memory per devices. Firewall configuration must be done in a
> > trusted
> > environment: for ARM architecture we plan to use OP-TEE + a trusted
> > application to do that.
>
> It's not just an ARM feature so any basis for this in the core code
> should be generic, whether its being enforced by ARM SDP, various Intel
> feature sets or even via a hypervisor.
>
> > I have try 2 "hacky" approachs with dma_buf:
> > - add a secure field in dma_buf structure and configure firewall in
> > dma_buf_{map/unmap}_attachment() functions.
>
> How is SDP not just another IOMMU. The only oddity here is that it
> happens to configure buffers the CPU can't touch and it has a control
> mechanism that is designed to cover big media corp type uses where the
> threat model is that the system owner is the enemy. Why does anything care
> about it being SDP, there are also generic cases this might be a useful
> optimisation (eg knowing the buffer isn't CPU touched so you can optimise
> cache flushing).
>
> The control mechanism is a device/platform detail as with any IOMMU. It
> doesn't matter who configures it or how, providing it happens.
>
> We do presumably need some small core DMA changes - anyone trying to map
> such a buffer into CPU space needs to get a warning or error but what
> else ?
>
> > >From buffer allocation point of view I also facing a problem because when
> > >v4l2
> > or drm/kms are exporting buffers by using dma_buf they don't attaching
> > themself on it and never call dma_buf_{map/unmap}_attachment(). This is not
> > an issue in those framework while it is how dma_buf exporters are
> > supposed to work.
>
> Which could be addressed if need be.
>
> So if "SDP" is just another IOMMU feature, just as stuff like IMR is on
> some x86 devices, and hypervisor enforced protection is on assorted
> platforms why do we need a special way to do it ? Is there anything
> actually needed beyond being able to tell the existing DMA code that this
> buffer won't be CPU touched and wiring it into the DMA operations for the
> platform ?
Iirc most of the dma api stuff gets unhappy when memory isn't struct page
backed. In i915 we do use sg tables everywhere though (even for memory not
backed by struct page, e.g. the "stolen" range the bios prereserves), but
we fill those out manually.
A possible generic design I see is to have a secure memory allocator
device which doesn nothing else but hand out dma-bufs. With that we can
hide the platform-specific allocation methods in there (some need to
allocate from carveouts, other just need to mark the pages specifically).
Also dma-buf has explicit methods for cpu access, which are allowed to
fail. And using the dma-buf attach tracking we can also reject dma to
devices which cannot access the secure memory. Given all that I think
going through the dma-buf interface but with a special-purpose allocator
seems to fit.
I'm not sure whether a special iommu is a good idea otoh: I'd expect that
for most devices the driver would need to decide about which iommu to pick
(or maybe keep track of some special flags for an extended dma_map
interface). At least looking at gpu drivers using iommus would require
special code, whereas fully hiding all this behind the dma-buf interface
should fit in much better.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[PATCH] drm/msm: Fix compil issue when DRM_MSM_FBDEV is disabled
On Wed, May 6, 2015 at 9:25 AM, Stephane Viau wrote:
> When CONFIG_DRM_MSM_FBDEV is not defined,
> CONFIG_DRM_KMS_FB_HELPER does not get selected and
> drm_fb_helper_*() helper functions are thus not available.
>
> This change fixes these link issues.
Hmm, didn't Archit start on making fbdev config option global and
adding nop-stubs for the case that it was disabled? I lost track of
where that was going..
BR,
-R
> Signed-off-by: Stephane Viau
> ---
> drivers/gpu/drm/msm/msm_drv.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> index 2b1218c..35380ec 100644
> --- a/drivers/gpu/drm/msm/msm_drv.c
> +++ b/drivers/gpu/drm/msm/msm_drv.c
> @@ -21,9 +21,11 @@
>
> static void msm_fb_output_poll_changed(struct drm_device *dev)
> {
> +#ifdef DRM_MSM_FBDEV
> struct msm_drm_private *priv = dev->dev_private;
> if (priv->fbdev)
> drm_fb_helper_hotplug_event(priv->fbdev);
> +#endif
> }
>
> static const struct drm_mode_config_funcs mode_config_funcs = {
> @@ -419,9 +421,11 @@ static void msm_preclose(struct drm_device *dev, struct
> drm_file *file)
>
> static void msm_lastclose(struct drm_device *dev)
> {
> +#ifdef DRM_MSM_FBDEV
> struct msm_drm_private *priv = dev->dev_private;
> if (priv->fbdev)
> drm_fb_helper_restore_fbdev_mode_unlocked(priv->fbdev);
> +#endif
> }
>
> static irqreturn_t msm_irq(int irq, void *arg)
> --
> Qualcomm Innovation Center, Inc.
>
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a
> Linux Foundation Collaborative Project
>
[PULL] topic/drm-misc
Hi Dave, rc2 is out, time to open drm-next. Bunch of drm_irq.c patches from Mario&me, magic/auth removal from David and a few other oddball things. Cheers, Daniel The following changes since commit 2b1193d5287004edfbf89407149a3159656f47f1: drm: fix trivial typo mistake (2015-04-14 12:41:03 +0200) are available in the git repository at: git://anongit.freedesktop.org/drm-intel tags/topic/drm-misc-2015-05-06 for you to fetch changes up to 4a324d33bfe95a279d4c7370d84087d3e773e799: drm: simplify master cleanup (2015-05-05 09:46:38 +0200) Daniel Vetter (3): drm/atomic-helper: Don't call atomic_update_plane when it stays off drm/vblank: Fixup and document timestamp update/read barriers drm/atomic-helper: Really recover pre-atomic plane/cursor behavior David Herrmann (3): drm: drop unused 'magicfree' list drm: simplify authentication management drm: simplify master cleanup Josef Holzmayr (1): DRM: Don't re-poll connector for disconnect Mario Kleiner (3): drm: Prevent invalid use of vblank_disable_immediate. (v2) drm: Zero out invalid vblank timestamp in drm_update_vblank_count. (v2) drm/qxl: Fix qxl_noop_get_vblank_counter() Oleg Drokin (1): drm: fix a memleak on mutex failure path Todd Previte (2): drm: Fix the 'native defer' message in drm_dp_i2c_do_msg() drm: Fix for DP CTS test 4.2.2.5 - I2C DEFER handling drivers/gpu/drm/drm_atomic_helper.c | 9 +- drivers/gpu/drm/drm_auth.c| 178 ++ drivers/gpu/drm/drm_dp_helper.c | 12 +- drivers/gpu/drm/drm_drv.c | 20 +--- drivers/gpu/drm/drm_fops.c| 7 +- drivers/gpu/drm/drm_internal.h| 1 - drivers/gpu/drm/drm_irq.c | 103 + drivers/gpu/drm/drm_modeset_lock.c| 8 +- drivers/gpu/drm/drm_probe_helper.c| 4 +- drivers/gpu/drm/i915/intel_atomic_plane.c | 4 - drivers/gpu/drm/qxl/qxl_drv.c | 2 +- include/drm/drmP.h| 14 ++- 12 files changed, 132 insertions(+), 230 deletions(-) -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
i915 dma_map_sg return value
On Tue, May 05, 2015 at 07:19:57PM +0200, Volker Vogelhuber wrote: > On 05.05.2015 17:51, Daniel Vetter wrote: > >On Tue, May 05, 2015 at 09:42:44AM +0200, Volker Vogelhuber wrote: > >>The documentation of the DMA-API writes the following about > >>dma_map_sg: > >> > >>"The implementation is free to merge several consecutive sglist entries > >>into one (e.g. if DMA mapping is done with PAGE_SIZE granularity, any > >>consecutive sglist entries can be merged into one provided the first one > >>ends and the second one starts on a page boundary - in fact this is a huge > >>advantage for cards which either cannot do scatter-gather or have very > >>limited number of scatter-gather entries) and returns the actual number > >>of sg entries it mapped them to." > >> > >>I wonder why the return value of dma_map_sg is not returned in any way > >>from i915_gem_map_dma_buf. It only uses the return value for error > >>checking. > >>Can one be sure that in case of the i915 the nents value of the scatter > >>gather table is always equal to the value returned by dma_map_sg? > >>I'm asking because I want to use the sg table returned by > >>i915_gem_map_dma_buf in my own kernel module and iterate over it > >>using for_each_sg. And the example in the documentation of the DMA-API > >>uses the return value of dma_map_sg when calling for_each_sg and not > >>nents and it explicitly mentions: > >> > >>"Then you should loop count times (note: this can be less than nents times)" > >Hm, not looking at the return value of dma_map_sg is also how we use it > >internally in i915_gem_gtt.c. Not sure why we get away with this ... > Maybe you can be sure that on systems where the i915 driver is > used no reduction of the nents will be done by dma_map_sg? Not sure that's true. But we do use for_each_sg_page, which might be immune to coallescing issue. But tbh I have no idea. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
component_unbind_all() splat..
Hey Russell, I just noticed a splat triggered by component_unbind_all() called from ->bind().. any opinions about whether component stuff should handle that case, or whether I should rearrange my error cleanup path to not component_unbind_all() in this case? [2.329829] msm 1a0.qcom,mdss_mdp: failed to allocate VRAM [2.334027] [ cut here ] [2.339754] WARNING: CPU: 1 PID: 1 at ../drivers/base/component.c:356 component_unbind.isra.2+0x5c/0x68() [2.344536] Modules linked in: [2.356918] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 4.0.0-dirty #5 [2.356996] Hardware name: Qualcomm Technologies, Inc. APQ 8016 SBC (DT) [2.363420] Call trace: [2.370110] [] dump_backtrace+0x0/0x118 [2.372276] [] show_stack+0x10/0x20 [2.377840] [] dump_stack+0x84/0xc4 [2.382865] [] warn_slowpath_common+0x94/0xd0 [2.387910] [] warn_slowpath_null+0x14/0x20 [2.393631] [] component_unbind.isra.2+0x58/0x68 [2.399543] [] component_unbind_all+0x84/0xc0 [2.405619] [] msm_unload+0x118/0x1a0 [2.411425] [] msm_load+0xe0/0x3b8 [2.416643] [] drm_dev_register+0xb8/0x110 [2.421843] [] drm_platform_init+0x44/0xe0 [2.427493] [] msm_drm_bind+0x18/0x28 [2.433125] [] try_to_bring_up_master.part.1+0xc0/0x108 [2.438087] [] component_master_add_with_match+0xb0/0x138 [2.445031] [] msm_pdev_probe+0x64/0x78 [2.451791] [] platform_drv_probe+0x48/0xb8 [2.457355] [] driver_probe_device+0x8c/0x248 [2.463074] [] __driver_attach+0x98/0xa0 [2.468811] [] bus_for_each_dev+0x5c/0xa0 [2.474366] [] driver_attach+0x1c/0x28 [2.479826] [] bus_add_driver+0x14c/0x208 [2.485390] [] driver_register+0x5c/0x120 [2.490679] [] __platform_driver_register+0x58/0x68 [2.496246] [] msm_drm_register+0x48/0x54 [2.502831] [] do_one_initcall+0x88/0x1a8 [2.508223] [] kernel_init_freeable+0x144/0x1ec [2.513769] [] kernel_init+0xc/0xe0 [2.520034] ---[ end trace 7e1590a270796f2f ]--- [2.525063] Unable to handle kernel NULL pointer dereference at virtual address 0028
[alsa-devel] [PATCH RFC v2 10/13] sound/core: add DRM ELD helper
On Tue, 2015-05-05 at 23:35 +0100, Mark Brown wrote: > On Sun, Apr 05, 2015 at 05:57:09PM +0200, Takashi Iwai wrote: > > At Thu, 02 Apr 2015 10:22:06 +0100, > > Russell King wrote: > > > > +config SND_PCM_ELD > > > + bool > > > I don't mind much adding this one, but a new Kconfig is always a > > question. I'd like to hear other's opinion, too. > > One point in favour of this is that there's been some discussion > recently of IoT applications with very low resource requirements that > might need audion functionality so it's likely that people will be > taking a look at trying to minimize code size for the audio stack. This > sort of configurability may well be useful for such applications. Fwiw, Keyon is looking at reducing runtime audio DRAM usage atm and will probably have to add other Kconfig options to the audio kernel config e.g. disable DPCM, disable DAPM, disable HW/SW params refinement, disable async audio ioctls, etc. Currently the kernel audio core and drivers are coming in at between 300k to 500k, which is a lot for a device with limited memory and very simple audio requirements. Liam
[PATCH] drm/panel: add lg4573 driver
The patch adds LG4573 parallel RGB panel driver with SPI control interface.
The driver uses drm_panel framework.
Signed-off-by: Heiko Schocher
---
.../devicetree/bindings/panel/lg,lg4573.txt| 42 +++
drivers/gpu/drm/panel/Kconfig | 8 +
drivers/gpu/drm/panel/Makefile | 1 +
drivers/gpu/drm/panel/panel-lg4573.c | 367 +
4 files changed, 418 insertions(+)
create mode 100644 Documentation/devicetree/bindings/panel/lg,lg4573.txt
create mode 100644 drivers/gpu/drm/panel/panel-lg4573.c
diff --git a/Documentation/devicetree/bindings/panel/lg,lg4573.txt
b/Documentation/devicetree/bindings/panel/lg,lg4573.txt
new file mode 100644
index 000..55f495d
--- /dev/null
+++ b/Documentation/devicetree/bindings/panel/lg,lg4573.txt
@@ -0,0 +1,42 @@
+LG LG4573 TFT liquid crystal display with SPI control bus
+
+Required properties:
+ - compatible: "lg4573"
+ - reg: address of the panel on SPI bus
+ - display-timings: timings for the connected panel according to [1]
+
+The panel must obey rules for SPI slave device specified in document [2].
+
+Optional properties:
+ - power-on-delay: delay after turning regulators on [ms]
+
+[1]: Documentation/devicetree/bindings/video/display-timing.txt
+[2]: Documentation/devicetree/bindings/spi/spi-bus.txt
+
+Example:
+
+ lcd_panel: display at 0 {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ compatible = "lg,lg4573";
+ spi-max-frequency = <1000>;
+ reset-gpios = <&gpio2 11 0>;
+ reg = <0>;
+ power-on-delay = <10>;
+ display-timings {
+ 480x800p57 {
+ native-mode;
+ clock-frequency = <2727>;
+ hactive = <480>;
+ vactive = <800>;
+ hfront-porch = <10>;
+ hback-porch = <59>;
+ hsync-len = <10>;
+ vback-porch = <15>;
+ vfront-porch = <15>;
+ vsync-len = <15>;
+ hsync-active = <1>;
+ vsync-active = <1>;
+ };
+ };
+ };
diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index 6d64c7b..29c3407 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -23,6 +23,14 @@ config DRM_PANEL_LD9040
depends on OF && SPI
select VIDEOMODE_HELPERS
+config DRM_PANEL_LG4573
+ tristate "LG4573 RGB/SPI panel"
+ depends on OF && SPI
+ select VIDEOMODE_HELPERS
+ help
+ Say Y here if you want to enable support for LG4573 RGB panel.
+ To compile this driver as a module, choose M here.
+
config DRM_PANEL_S6E8AA0
tristate "S6E8AA0 DSI video mode panel"
depends on OF
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index 4b2a043..715b95d 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -1,4 +1,5 @@
obj-$(CONFIG_DRM_PANEL_SIMPLE) += panel-simple.o
obj-$(CONFIG_DRM_PANEL_LD9040) += panel-ld9040.o
+obj-$(CONFIG_DRM_PANEL_LG4573) += panel-lg4573.o
obj-$(CONFIG_DRM_PANEL_S6E8AA0) += panel-s6e8aa0.o
obj-$(CONFIG_DRM_PANEL_SHARP_LQ101R1SX01) += panel-sharp-lq101r1sx01.o
diff --git a/drivers/gpu/drm/panel/panel-lg4573.c
b/drivers/gpu/drm/panel/panel-lg4573.c
new file mode 100644
index 000..9d5e5a5
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-lg4573.c
@@ -0,0 +1,367 @@
+/*
+ *
+ * Copyright (C) 2015 Heiko Schocher
+ *
+ * from:
+ * drivers/gpu/drm/panel/panel-ld9040.c
+ * ld9040 AMOLED LCD drm_panel driver.
+ *
+ * Copyright (c) 2014 Samsung Electronics Co., Ltd
+ * Derived from drivers/video/backlight/ld9040.c
+ *
+ * Andrzej Hajda
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+*/
+
+#include
+#include
+
+#include
+#include
+#include
+
+#include
+#include
+#include
+
+struct lg4573 {
+ struct device *dev;
+ struct drm_panel panel;
+ u32 power_on_delay;
+ struct videomode vm;
+};
+
+static inline struct lg4573 *panel_to_lg4573(struct drm_panel *panel)
+{
+ return container_of(panel, struct lg4573, panel);
+}
+
+static int lg4573_spi_write_u16(struct lg4573 *ctx, u16 data)
+{
+ struct spi_device *spi = to_spi_device(ctx->dev);
+ struct spi_transfer xfer = {
+ .len= 2,
+ };
+ struct spi_message msg;
+ u16 temp = htons(data);
+
+ dev_dbg(ctx->dev, "writing data: %x\n", data);
+ xfer.tx_buf = &temp;
+ spi_message_init(&msg);
+ spi_mess
[PATCH 6/8] pwm: crc: Add Crystalcove (CRC) PWM driver
On Tue, 2015-05-05 at 15:08 +0530, Shobhit Kumar wrote: > The Crystalcove PMIC controls PWM signals and this driver exports that > capability as a PWM chip driver. This is platform device implementtaion > of the drivers/mfd cell device for CRC PMIC > > v2: Use the existing config callback with duty_ns and period_ns(Thierry) > > v3: Correct the subject line (Lee jones) > > CC: Samuel Ortiz > Cc: Linus Walleij > Cc: Alexandre Courbot > Cc: Thierry Reding > Signed-off-by: Shobhit Kumar The same comments can be made as for v2, see http://lkml.kernel.org/r/1430428322.2187.24.camel at x220 . Maybe you didn't receive that message. It could also be that you think my comments were invalid, or too vague, or whatever. Please say so, because then I don't have to bother you again when you send out v4. Thanks, Paul Bolle
[PATCH 1/3] drm/edid: Fix up DMT modes
On Wed, May 6, 2015 at 9:36 AM, Ville Syrjälä
wrote:
> On Thu, Apr 02, 2015 at 05:02:09PM +0300, ville.syrjala at linux.intel.com
> wrote:
>> From: Ville Syrjälä
>>
>> Liu Lei noticed that our 1856x1392 at 75Hz DMT mode doesn't match the spec.
>> Fix that up, and also fix up a few other inconsistencies I discovered
>> by parsing the spec (DMT version 1.0, revision 13) and comparing the
>> results to our current DMT mode table.
>>
>> Also clean up the indentation mess for the 1024x768i mode.
>>
>> Cc: "liu,lei"
>> Signed-off-by: Ville Syrjälä
>
> Ping. Anyone interested in DMT modes that match the spec?
I started reviewing this series when you sent it out, but I guess I
got distracted and never sent out my rb:
Reviewed-by: Alex Deucher
>
>> ---
>> drivers/gpu/drm/drm_edid.c | 10 +-
>> 1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> index 53bc7a6..567c7ed 100644
>> --- a/drivers/gpu/drm/drm_edid.c
>> +++ b/drivers/gpu/drm/drm_edid.c
>> @@ -160,7 +160,7 @@ static const struct drm_display_mode drm_dmt_modes[] = {
>> DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_PVSYNC) },
>> /* 640x480 at 60Hz */
>> { DRM_MODE("640x480", DRM_MODE_TYPE_DRIVER, 25175, 640, 656,
>> -752, 800, 0, 480, 489, 492, 525, 0,
>> +752, 800, 0, 480, 490, 492, 525, 0,
>> DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC) },
>> /* 640x480 at 72Hz */
>> { DRM_MODE("640x480", DRM_MODE_TYPE_DRIVER, 31500, 640, 664,
>> @@ -206,7 +206,7 @@ static const struct drm_display_mode drm_dmt_modes[] = {
>> { DRM_MODE("1024x768i", DRM_MODE_TYPE_DRIVER, 44900, 1024, 1032,
>> 1208, 1264, 0, 768, 768, 772, 817, 0,
>> DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC |
>> - DRM_MODE_FLAG_INTERLACE) },
>> +DRM_MODE_FLAG_INTERLACE) },
>> /* 1024x768 at 60Hz */
>> { DRM_MODE("1024x768", DRM_MODE_TYPE_DRIVER, 65000, 1024, 1048,
>> 1184, 1344, 0, 768, 771, 777, 806, 0,
>> @@ -242,7 +242,7 @@ static const struct drm_display_mode drm_dmt_modes[] = {
>> /* 1280x768 at 75Hz */
>> { DRM_MODE("1280x768", DRM_MODE_TYPE_DRIVER, 102250, 1280, 1360,
>> 1488, 1696, 0, 768, 771, 778, 805, 0,
>> -DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NVSYNC) },
>> +DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_PVSYNC) },
>> /* 1280x768 at 85Hz */
>> { DRM_MODE("1280x768", DRM_MODE_TYPE_DRIVER, 117500, 1280, 1360,
>> 1496, 1712, 0, 768, 771, 778, 809, 0,
>> @@ -258,7 +258,7 @@ static const struct drm_display_mode drm_dmt_modes[] = {
>> /* 1280x800 at 60Hz */
>> { DRM_MODE("1280x800", DRM_MODE_TYPE_DRIVER, 83500, 1280, 1352,
>> 1480, 1680, 0, 800, 803, 809, 831, 0,
>> -DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NVSYNC) },
>> +DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_PVSYNC) },
>> /* 1280x800 at 75Hz */
>> { DRM_MODE("1280x800", DRM_MODE_TYPE_DRIVER, 106500, 1280, 1360,
>> 1488, 1696, 0, 800, 803, 809, 838, 0,
>> @@ -409,7 +409,7 @@ static const struct drm_display_mode drm_dmt_modes[] = {
>> DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_PVSYNC) },
>> /* 1856x1392 at 75Hz */
>> { DRM_MODE("1856x1392", DRM_MODE_TYPE_DRIVER, 288000, 1856, 1984,
>> -2208, 2560, 0, 1392, 1395, 1399, 1500, 0,
>> +2208, 2560, 0, 1392, 1393, 1396, 1500, 0,
>> DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_PVSYNC) },
>> /* 1856x1392 at 120Hz RB */
>> { DRM_MODE("1856x1392", DRM_MODE_TYPE_DRIVER, 356500, 1856, 1904,
>> --
>> 2.0.5
>
> --
> Ville Syrjälä
> Intel OTC
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
i915 dma_map_sg return value
On Wed, May 06, 2015 at 10:24:47AM +0200, Daniel Vetter wrote: > On Tue, May 05, 2015 at 07:19:57PM +0200, Volker Vogelhuber wrote: > > On 05.05.2015 17:51, Daniel Vetter wrote: > > >On Tue, May 05, 2015 at 09:42:44AM +0200, Volker Vogelhuber wrote: > > >>The documentation of the DMA-API writes the following about > > >>dma_map_sg: > > >> > > >>"The implementation is free to merge several consecutive sglist entries > > >>into one (e.g. if DMA mapping is done with PAGE_SIZE granularity, any > > >>consecutive sglist entries can be merged into one provided the first one > > >>ends and the second one starts on a page boundary - in fact this is a huge > > >>advantage for cards which either cannot do scatter-gather or have very > > >>limited number of scatter-gather entries) and returns the actual number > > >>of sg entries it mapped them to." > > >> > > >>I wonder why the return value of dma_map_sg is not returned in any way > > >>from i915_gem_map_dma_buf. It only uses the return value for error > > >>checking. > > >>Can one be sure that in case of the i915 the nents value of the scatter > > >>gather table is always equal to the value returned by dma_map_sg? > > >>I'm asking because I want to use the sg table returned by > > >>i915_gem_map_dma_buf in my own kernel module and iterate over it > > >>using for_each_sg. And the example in the documentation of the DMA-API > > >>uses the return value of dma_map_sg when calling for_each_sg and not > > >>nents and it explicitly mentions: > > >> > > >>"Then you should loop count times (note: this can be less than nents > > >>times)" > > >Hm, not looking at the return value of dma_map_sg is also how we use it > > >internally in i915_gem_gtt.c. Not sure why we get away with this ... > > Maybe you can be sure that on systems where the i915 driver is > > used no reduction of the nents will be done by dma_map_sg? > > Not sure that's true. But we do use for_each_sg_page, which might be > immune to coallescing issue. But tbh I have no idea. It is. nents is entirely redundant in the loop since we terminate with the sentinel marker in the sg and iterate over amalgamated sg entries. However, we probably should make sure that nents stays in sync in case some other routine depends on its correctness. -Chris -- Chris Wilson, Intel Open Source Technology Centre
[PATCH 9/9] drm/exynos: Convert g2d_userptr_get_dma_addr() to use get_vaddr_frames()
Convert g2d_userptr_get_dma_addr() to pin pages using get_vaddr_frames().
This removes the knowledge about vmas and mmap_sem locking from exynos
driver. Also it fixes a problem that the function has been mapping user
provided address without holding mmap_sem.
Signed-off-by: Jan Kara
---
drivers/gpu/drm/exynos/exynos_drm_g2d.c | 89 ++
drivers/gpu/drm/exynos/exynos_drm_gem.c | 97 -
2 files changed, 29 insertions(+), 157 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_g2d.c
b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
index 81a250830808..265519c0fe2d 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_g2d.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
@@ -190,10 +190,8 @@ struct g2d_cmdlist_userptr {
dma_addr_t dma_addr;
unsigned long userptr;
unsigned long size;
- struct page **pages;
- unsigned intnpages;
+ struct frame_vector *vec;
struct sg_table *sgt;
- struct vm_area_struct *vma;
atomic_trefcount;
boolin_pool;
boolout_of_list;
@@ -363,6 +361,7 @@ static void g2d_userptr_put_dma_addr(struct drm_device
*drm_dev,
{
struct g2d_cmdlist_userptr *g2d_userptr =
(struct g2d_cmdlist_userptr *)obj;
+ struct page **pages;
if (!obj)
return;
@@ -382,19 +381,21 @@ out:
exynos_gem_unmap_sgt_from_dma(drm_dev, g2d_userptr->sgt,
DMA_BIDIRECTIONAL);
- exynos_gem_put_pages_to_userptr(g2d_userptr->pages,
- g2d_userptr->npages,
- g2d_userptr->vma);
+ pages = frame_vector_pages(g2d_userptr->vec);
+ if (!IS_ERR(pages)) {
+ int i;
- exynos_gem_put_vma(g2d_userptr->vma);
+ for (i = 0; i < frame_vector_count(g2d_userptr->vec); i++)
+ set_page_dirty_lock(pages[i]);
+ }
+ put_vaddr_frames(g2d_userptr->vec);
+ frame_vector_destroy(g2d_userptr->vec);
if (!g2d_userptr->out_of_list)
list_del_init(&g2d_userptr->list);
sg_free_table(g2d_userptr->sgt);
kfree(g2d_userptr->sgt);
-
- drm_free_large(g2d_userptr->pages);
kfree(g2d_userptr);
}
@@ -413,6 +414,7 @@ static dma_addr_t *g2d_userptr_get_dma_addr(struct
drm_device *drm_dev,
struct vm_area_struct *vma;
unsigned long start, end;
unsigned int npages, offset;
+ struct frame_vector *vec;
int ret;
if (!size) {
@@ -456,65 +458,37 @@ static dma_addr_t *g2d_userptr_get_dma_addr(struct
drm_device *drm_dev,
return ERR_PTR(-ENOMEM);
atomic_set(&g2d_userptr->refcount, 1);
+ g2d_userptr->size = size;
start = userptr & PAGE_MASK;
offset = userptr & ~PAGE_MASK;
end = PAGE_ALIGN(userptr + size);
npages = (end - start) >> PAGE_SHIFT;
- g2d_userptr->npages = npages;
-
- pages = drm_calloc_large(npages, sizeof(struct page *));
- if (!pages) {
- DRM_ERROR("failed to allocate pages.\n");
- ret = -ENOMEM;
+ vec = g2d_userptr->vec = frame_vector_create(npages);
+ if (!vec)
goto err_free;
- }
- down_read(¤t->mm->mmap_sem);
- vma = find_vma(current->mm, userptr);
- if (!vma) {
- up_read(¤t->mm->mmap_sem);
- DRM_ERROR("failed to get vm region.\n");
+ ret = get_vaddr_frames(start, npages, 1, 1, vec);
+ if (ret != npages) {
+ DRM_ERROR("failed to get user pages from userptr.\n");
+ if (ret < 0)
+ goto err_destroy_framevec;
ret = -EFAULT;
- goto err_free_pages;
+ goto err_put_framevec;
}
-
- if (vma->vm_end < userptr + size) {
- up_read(¤t->mm->mmap_sem);
- DRM_ERROR("vma is too small.\n");
+ if (frame_vector_to_pages(vec) < 0) {
ret = -EFAULT;
- goto err_free_pages;
+ goto err_put_framevec;
}
- g2d_userptr->vma = exynos_gem_get_vma(vma);
- if (!g2d_userptr->vma) {
- up_read(¤t->mm->mmap_sem);
- DRM_ERROR("failed to copy vma.\n");
- ret = -ENOMEM;
- goto err_free_pages;
- }
-
- g2d_userptr->size = size;
-
- ret = exynos_gem_get_pages_from_userptr(start & PAGE_MASK,
- npages, pages, vma);
- if (ret < 0) {
- up_read(¤t->mm->mmap_sem);
- DRM_ERROR("failed to get user pages from userptr.\n");
- goto err_put_vma;
- }
-
- up_read(¤t->mm->mmap_sem);
- g2d_userptr->p
[PATCH 8/9] media: vb2: Remove unused functions
Conversion to the use of pinned pfns made some functions unused. Remove
them. Also there's no need to lock mmap_sem in __buf_prepare() anymore.
Acked-by: Marek Szyprowski
Tested-by: Marek Szyprowski
Signed-off-by: Jan Kara
---
drivers/media/v4l2-core/videobuf2-memops.c | 114 -
include/media/videobuf2-memops.h | 6 --
2 files changed, 120 deletions(-)
diff --git a/drivers/media/v4l2-core/videobuf2-memops.c
b/drivers/media/v4l2-core/videobuf2-memops.c
index 0ec186d41b9b..48c6a49c4928 100644
--- a/drivers/media/v4l2-core/videobuf2-memops.c
+++ b/drivers/media/v4l2-core/videobuf2-memops.c
@@ -23,120 +23,6 @@
#include
/**
- * vb2_get_vma() - acquire and lock the virtual memory area
- * @vma: given virtual memory area
- *
- * This function attempts to acquire an area mapped in the userspace for
- * the duration of a hardware operation. The area is "locked" by performing
- * the same set of operation that are done when process calls fork() and
- * memory areas are duplicated.
- *
- * Returns a copy of a virtual memory region on success or NULL.
- */
-struct vm_area_struct *vb2_get_vma(struct vm_area_struct *vma)
-{
- struct vm_area_struct *vma_copy;
-
- vma_copy = kmalloc(sizeof(*vma_copy), GFP_KERNEL);
- if (vma_copy == NULL)
- return NULL;
-
- if (vma->vm_ops && vma->vm_ops->open)
- vma->vm_ops->open(vma);
-
- if (vma->vm_file)
- get_file(vma->vm_file);
-
- memcpy(vma_copy, vma, sizeof(*vma));
-
- vma_copy->vm_mm = NULL;
- vma_copy->vm_next = NULL;
- vma_copy->vm_prev = NULL;
-
- return vma_copy;
-}
-EXPORT_SYMBOL_GPL(vb2_get_vma);
-
-/**
- * vb2_put_userptr() - release a userspace virtual memory area
- * @vma: virtual memory region associated with the area to be released
- *
- * This function releases the previously acquired memory area after a hardware
- * operation.
- */
-void vb2_put_vma(struct vm_area_struct *vma)
-{
- if (!vma)
- return;
-
- if (vma->vm_ops && vma->vm_ops->close)
- vma->vm_ops->close(vma);
-
- if (vma->vm_file)
- fput(vma->vm_file);
-
- kfree(vma);
-}
-EXPORT_SYMBOL_GPL(vb2_put_vma);
-
-/**
- * vb2_get_contig_userptr() - lock physically contiguous userspace mapped
memory
- * @vaddr: starting virtual address of the area to be verified
- * @size: size of the area
- * @res_paddr: will return physical address for the given vaddr
- * @res_vma: will return locked copy of struct vm_area for the given area
- *
- * This function will go through memory area of size @size mapped at @vaddr and
- * verify that the underlying physical pages are contiguous. If they are
- * contiguous the virtual memory area is locked and a @res_vma is filled with
- * the copy and @res_pa set to the physical address of the buffer.
- *
- * Returns 0 on success.
- */
-int vb2_get_contig_userptr(unsigned long vaddr, unsigned long size,
- struct vm_area_struct **res_vma, dma_addr_t *res_pa)
-{
- struct mm_struct *mm = current->mm;
- struct vm_area_struct *vma;
- unsigned long offset, start, end;
- unsigned long this_pfn, prev_pfn;
- dma_addr_t pa = 0;
-
- start = vaddr;
- offset = start & ~PAGE_MASK;
- end = start + size;
-
- vma = find_vma(mm, start);
-
- if (vma == NULL || vma->vm_end < end)
- return -EFAULT;
-
- for (prev_pfn = 0; start < end; start += PAGE_SIZE) {
- int ret = follow_pfn(vma, start, &this_pfn);
- if (ret)
- return ret;
-
- if (prev_pfn == 0)
- pa = this_pfn << PAGE_SHIFT;
- else if (this_pfn != prev_pfn + 1)
- return -EFAULT;
-
- prev_pfn = this_pfn;
- }
-
- /*
-* Memory is contigous, lock vma and return to the caller
-*/
- *res_vma = vb2_get_vma(vma);
- if (*res_vma == NULL)
- return -ENOMEM;
-
- *res_pa = pa + offset;
- return 0;
-}
-EXPORT_SYMBOL_GPL(vb2_get_contig_userptr);
-
-/**
* vb2_create_framevec() - map virtual addresses to pfns
* @start: Virtual user address where we start mapping
* @length:Length of a range to map
diff --git a/include/media/videobuf2-memops.h b/include/media/videobuf2-memops.h
index 2f0564ff5f31..830b5239fd8b 100644
--- a/include/media/videobuf2-memops.h
+++ b/include/media/videobuf2-memops.h
@@ -31,12 +31,6 @@ struct vb2_vmarea_handler {
extern const struct vm_operations_struct vb2_common_vm_ops;
-int vb2_get_contig_userptr(unsigned long vaddr, unsigned long size,
- struct vm_area_struct **res_vma, dma_addr_t *res_pa);
-
-struct vm_area_struct *vb2_get_vma(struct vm_area_struct *vma);
-void vb2_put_vma(struct vm_area_struct *vma);
-
struct frame_vector *vb2_create_framevec(unsigned long start
[PATCH 7/9] media: vb2: Convert vb2_dc_get_userptr() to use frame vector
Convert vb2_dc_get_userptr() to use frame vector infrastructure. When we
are doing that there's no need to allocate page array and some code can
be simplified.
Acked-by: Marek Szyprowski
Tested-by: Marek Szyprowski
Signed-off-by: Jan Kara
---
drivers/media/v4l2-core/videobuf2-dma-contig.c | 214 -
1 file changed, 34 insertions(+), 180 deletions(-)
diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c
b/drivers/media/v4l2-core/videobuf2-dma-contig.c
index 620c4aa78881..e6cea452302b 100644
--- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
@@ -32,15 +32,13 @@ struct vb2_dc_buf {
dma_addr_t dma_addr;
enum dma_data_direction dma_dir;
struct sg_table *dma_sgt;
+ struct frame_vector *vec;
/* MMAP related */
struct vb2_vmarea_handler handler;
atomic_trefcount;
struct sg_table *sgt_base;
- /* USERPTR related */
- struct vm_area_struct *vma;
-
/* DMABUF related */
struct dma_buf_attachment *db_attach;
};
@@ -49,24 +47,6 @@ struct vb2_dc_buf {
/*scatterlist table functions*/
/*/
-
-static void vb2_dc_sgt_foreach_page(struct sg_table *sgt,
- void (*cb)(struct page *pg))
-{
- struct scatterlist *s;
- unsigned int i;
-
- for_each_sg(sgt->sgl, s, sgt->orig_nents, i) {
- struct page *page = sg_page(s);
- unsigned int n_pages = PAGE_ALIGN(s->offset + s->length)
- >> PAGE_SHIFT;
- unsigned int j;
-
- for (j = 0; j < n_pages; ++j, ++page)
- cb(page);
- }
-}
-
static unsigned long vb2_dc_get_contiguous_size(struct sg_table *sgt)
{
struct scatterlist *s;
@@ -429,92 +409,12 @@ static struct dma_buf *vb2_dc_get_dmabuf(void *buf_priv,
unsigned long flags)
/* callbacks for USERPTR buffers */
/*/
-static inline int vma_is_io(struct vm_area_struct *vma)
-{
- return !!(vma->vm_flags & (VM_IO | VM_PFNMAP));
-}
-
-static int vb2_dc_get_user_pfn(unsigned long start, int n_pages,
- struct vm_area_struct *vma, unsigned long *res)
-{
- unsigned long pfn, start_pfn, prev_pfn;
- unsigned int i;
- int ret;
-
- if (!vma_is_io(vma))
- return -EFAULT;
-
- ret = follow_pfn(vma, start, &pfn);
- if (ret)
- return ret;
-
- start_pfn = pfn;
- start += PAGE_SIZE;
-
- for (i = 1; i < n_pages; ++i, start += PAGE_SIZE) {
- prev_pfn = pfn;
- ret = follow_pfn(vma, start, &pfn);
-
- if (ret) {
- pr_err("no page for address %lu\n", start);
- return ret;
- }
- if (pfn != prev_pfn + 1)
- return -EINVAL;
- }
-
- *res = start_pfn;
- return 0;
-}
-
-static int vb2_dc_get_user_pages(unsigned long start, struct page **pages,
- int n_pages, struct vm_area_struct *vma,
- enum dma_data_direction dma_dir)
-{
- if (vma_is_io(vma)) {
- unsigned int i;
-
- for (i = 0; i < n_pages; ++i, start += PAGE_SIZE) {
- unsigned long pfn;
- int ret = follow_pfn(vma, start, &pfn);
-
- if (!pfn_valid(pfn))
- return -EINVAL;
-
- if (ret) {
- pr_err("no page for address %lu\n", start);
- return ret;
- }
- pages[i] = pfn_to_page(pfn);
- }
- } else {
- int n;
-
- n = get_user_pages(current, current->mm, start & PAGE_MASK,
- n_pages, dma_dir == DMA_FROM_DEVICE, 1, pages, NULL);
- /* negative error means that no page was pinned */
- n = max(n, 0);
- if (n != n_pages) {
- pr_err("got only %d of %d user pages\n", n, n_pages);
- while (n)
- put_page(pages[--n]);
- return -EFAULT;
- }
- }
-
- return 0;
-}
-
-static void vb2_dc_put_dirty_page(struct page *page)
-{
- set_page_dirty_lock(page);
- put_page(page);
-}
-
static void vb2_dc_put_userptr(void *buf_priv)
{
struct vb2_dc_buf *buf = buf_priv;
struct sg_table *sgt = buf->dma_sgt;
+ int i;
+ struct page **pages;
if (sgt) {
DEFINE_DMA_ATTRS(attrs);
@@ -526,15 +426,15 @@ static void vb2_dc_put_userptr(void *buf_priv)
*/
dma_unmap_sg_attrs(buf->dev, sgt->
[PATCH 6/9] media: vb2: Convert vb2_vmalloc_get_userptr() to use frame vector
Convert vb2_vmalloc_get_userptr() to use frame vector infrastructure.
When we are doing that there's no need to allocate page array and some
code can be simplified.
Acked-by: Marek Szyprowski
Tested-by: Marek Szyprowski
Signed-off-by: Jan Kara
---
drivers/media/v4l2-core/videobuf2-vmalloc.c | 94 +++--
1 file changed, 36 insertions(+), 58 deletions(-)
diff --git a/drivers/media/v4l2-core/videobuf2-vmalloc.c
b/drivers/media/v4l2-core/videobuf2-vmalloc.c
index 0ba40be21ebd..d2ce81fa2cdf 100644
--- a/drivers/media/v4l2-core/videobuf2-vmalloc.c
+++ b/drivers/media/v4l2-core/videobuf2-vmalloc.c
@@ -23,11 +23,9 @@
struct vb2_vmalloc_buf {
void*vaddr;
- struct page **pages;
- struct vm_area_struct *vma;
+ struct frame_vector *vec;
enum dma_data_direction dma_dir;
unsigned long size;
- unsigned intn_pages;
atomic_trefcount;
struct vb2_vmarea_handler handler;
struct dma_buf *dbuf;
@@ -76,10 +74,8 @@ static void *vb2_vmalloc_get_userptr(void *alloc_ctx,
unsigned long vaddr,
enum dma_data_direction dma_dir)
{
struct vb2_vmalloc_buf *buf;
- unsigned long first, last;
- int n_pages, offset;
- struct vm_area_struct *vma;
- dma_addr_t physp;
+ struct frame_vector *vec;
+ int n_pages, offset, i;
buf = kzalloc(sizeof(*buf), GFP_KERNEL);
if (!buf)
@@ -88,53 +84,36 @@ static void *vb2_vmalloc_get_userptr(void *alloc_ctx,
unsigned long vaddr,
buf->dma_dir = dma_dir;
offset = vaddr & ~PAGE_MASK;
buf->size = size;
-
- down_read(¤t->mm->mmap_sem);
- vma = find_vma(current->mm, vaddr);
- if (vma && (vma->vm_flags & VM_PFNMAP) && (vma->vm_pgoff)) {
- if (vb2_get_contig_userptr(vaddr, size, &vma, &physp))
- goto fail_pages_array_alloc;
- buf->vma = vma;
- buf->vaddr = (__force void *)ioremap_nocache(physp, size);
- if (!buf->vaddr)
- goto fail_pages_array_alloc;
+ vec = vb2_create_framevec(vaddr, size, dma_dir == DMA_FROM_DEVICE);
+ if (IS_ERR(vec))
+ goto fail_pfnvec_create;
+ buf->vec = vec;
+ n_pages = frame_vector_count(vec);
+ if (frame_vector_to_pages(vec) < 0) {
+ unsigned long *nums = frame_vector_pfns(vec);
+
+ /*
+* We cannot get page pointers for these pfns. Check memory is
+* physically contiguous and use direct mapping.
+*/
+ for (i = 1; i < n_pages; i++)
+ if (nums[i-1] + 1 != nums[i])
+ goto fail_map;
+ buf->vaddr = (__force void *)
+ ioremap_nocache(nums[0] << PAGE_SHIFT, size);
} else {
- first = vaddr >> PAGE_SHIFT;
- last = (vaddr + size - 1) >> PAGE_SHIFT;
- buf->n_pages = last - first + 1;
- buf->pages = kzalloc(buf->n_pages * sizeof(struct page *),
-GFP_KERNEL);
- if (!buf->pages)
- goto fail_pages_array_alloc;
-
- /* current->mm->mmap_sem is taken by videobuf2 core */
- n_pages = get_user_pages(current, current->mm,
-vaddr & PAGE_MASK, buf->n_pages,
-dma_dir == DMA_FROM_DEVICE,
-1, /* force */
-buf->pages, NULL);
- if (n_pages != buf->n_pages)
- goto fail_get_user_pages;
-
- buf->vaddr = vm_map_ram(buf->pages, buf->n_pages, -1,
+ buf->vaddr = vm_map_ram(frame_vector_pages(vec), n_pages, -1,
PAGE_KERNEL);
- if (!buf->vaddr)
- goto fail_get_user_pages;
}
- up_read(¤t->mm->mmap_sem);
+ if (!buf->vaddr)
+ goto fail_map;
buf->vaddr += offset;
return buf;
-fail_get_user_pages:
- pr_debug("get_user_pages requested/got: %d/%d]\n", n_pages,
-buf->n_pages);
- while (--n_pages >= 0)
- put_page(buf->pages[n_pages]);
- kfree(buf->pages);
-
-fail_pages_array_alloc:
- up_read(¤t->mm->mmap_sem);
+fail_map:
+ vb2_destroy_framevec(vec);
+fail_pfnvec_create:
kfree(buf);
return NULL;
@@ -145,22 +124,21 @@ static void vb2_vmalloc_put_userptr(void *buf_priv)
struct vb2_vmalloc_buf *buf = buf_priv;
unsigned long vaddr = (unsigned long)buf->vaddr & PAGE_MASK;
unsigned int i;
+ struct page **pag
[PATCH 5/9] media: vb2: Convert vb2_dma_sg_get_userptr() to use frame vector
Acked-by: Marek Szyprowski
Tested-by: Marek Szyprowski
Signed-off-by: Jan Kara
---
drivers/media/v4l2-core/videobuf2-dma-sg.c | 97 +-
1 file changed, 15 insertions(+), 82 deletions(-)
diff --git a/drivers/media/v4l2-core/videobuf2-dma-sg.c
b/drivers/media/v4l2-core/videobuf2-dma-sg.c
index afd4b514affc..4ee1b3fbfe2a 100644
--- a/drivers/media/v4l2-core/videobuf2-dma-sg.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-sg.c
@@ -38,6 +38,7 @@ struct vb2_dma_sg_buf {
struct device *dev;
void*vaddr;
struct page **pages;
+ struct frame_vector *vec;
int offset;
enum dma_data_direction dma_dir;
struct sg_table sg_table;
@@ -51,7 +52,6 @@ struct vb2_dma_sg_buf {
unsigned intnum_pages;
atomic_trefcount;
struct vb2_vmarea_handler handler;
- struct vm_area_struct *vma;
struct dma_buf_attachment *db_attach;
};
@@ -224,25 +224,17 @@ static void vb2_dma_sg_finish(void *buf_priv)
dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
}
-static inline int vma_is_io(struct vm_area_struct *vma)
-{
- return !!(vma->vm_flags & (VM_IO | VM_PFNMAP));
-}
-
static void *vb2_dma_sg_get_userptr(void *alloc_ctx, unsigned long vaddr,
unsigned long size,
enum dma_data_direction dma_dir)
{
struct vb2_dma_sg_conf *conf = alloc_ctx;
struct vb2_dma_sg_buf *buf;
- unsigned long first, last;
- int num_pages_from_user;
- struct vm_area_struct *vma;
struct sg_table *sgt;
DEFINE_DMA_ATTRS(attrs);
+ struct frame_vector *vec;
dma_set_attr(DMA_ATTR_SKIP_CPU_SYNC, &attrs);
-
buf = kzalloc(sizeof *buf, GFP_KERNEL);
if (!buf)
return NULL;
@@ -253,63 +245,19 @@ static void *vb2_dma_sg_get_userptr(void *alloc_ctx,
unsigned long vaddr,
buf->offset = vaddr & ~PAGE_MASK;
buf->size = size;
buf->dma_sgt = &buf->sg_table;
+ vec = vb2_create_framevec(vaddr, size, buf->dma_dir == DMA_FROM_DEVICE);
+ if (IS_ERR(vec))
+ goto userptr_fail_pfnvec;
+ buf->vec = vec;
- first = (vaddr & PAGE_MASK) >> PAGE_SHIFT;
- last = ((vaddr + size - 1) & PAGE_MASK) >> PAGE_SHIFT;
- buf->num_pages = last - first + 1;
-
- buf->pages = kzalloc(buf->num_pages * sizeof(struct page *),
-GFP_KERNEL);
- if (!buf->pages)
- goto userptr_fail_alloc_pages;
-
- down_read(¤t->mm->mmap_sem);
- vma = find_vma(current->mm, vaddr);
- if (!vma) {
- dprintk(1, "no vma for address %lu\n", vaddr);
- goto userptr_fail_find_vma;
- }
-
- if (vma->vm_end < vaddr + size) {
- dprintk(1, "vma at %lu is too small for %lu bytes\n",
- vaddr, size);
- goto userptr_fail_find_vma;
- }
-
- buf->vma = vb2_get_vma(vma);
- if (!buf->vma) {
- dprintk(1, "failed to copy vma\n");
- goto userptr_fail_find_vma;
- }
-
- if (vma_is_io(buf->vma)) {
- for (num_pages_from_user = 0;
-num_pages_from_user < buf->num_pages;
-++num_pages_from_user, vaddr += PAGE_SIZE) {
- unsigned long pfn;
-
- if (follow_pfn(vma, vaddr, &pfn)) {
- dprintk(1, "no page for address %lu\n", vaddr);
- break;
- }
- buf->pages[num_pages_from_user] = pfn_to_page(pfn);
- }
- } else
- num_pages_from_user = get_user_pages(current, current->mm,
-vaddr & PAGE_MASK,
-buf->num_pages,
-buf->dma_dir == DMA_FROM_DEVICE,
-1, /* force */
-buf->pages,
-NULL);
- up_read(¤t->mm->mmap_sem);
-
- if (num_pages_from_user != buf->num_pages)
- goto userptr_fail_get_user_pages;
+ buf->pages = frame_vector_pages(vec);
+ if (IS_ERR(buf->pages))
+ goto userptr_fail_sgtable;
+ buf->num_pages = frame_vector_count(vec);
if (sg_alloc_table_from_pages(buf->dma_sgt, buf->pages,
buf->num_pages, buf->offset, size, 0))
- goto userptr_fail_alloc_table_from_pages;
+ goto userptr_fail_sgtable;
sgt = &buf->sg_table;
/*
@@ -323,19 +271,9 @@ static void *vb2_
[PATCH 4/9] vb2: Provide helpers for mapping virtual addresses
Provide simple helper functions to map virtual address range into an
array of pfns / pages.
Acked-by: Marek Szyprowski
Tested-by: Marek Szyprowski
Signed-off-by: Jan Kara
---
drivers/media/v4l2-core/videobuf2-memops.c | 58 ++
include/media/videobuf2-memops.h | 5 +++
2 files changed, 63 insertions(+)
diff --git a/drivers/media/v4l2-core/videobuf2-memops.c
b/drivers/media/v4l2-core/videobuf2-memops.c
index 81c1ad8b2cf1..0ec186d41b9b 100644
--- a/drivers/media/v4l2-core/videobuf2-memops.c
+++ b/drivers/media/v4l2-core/videobuf2-memops.c
@@ -137,6 +137,64 @@ int vb2_get_contig_userptr(unsigned long vaddr, unsigned
long size,
EXPORT_SYMBOL_GPL(vb2_get_contig_userptr);
/**
+ * vb2_create_framevec() - map virtual addresses to pfns
+ * @start: Virtual user address where we start mapping
+ * @length:Length of a range to map
+ * @write: Should we map for writing into the area
+ *
+ * This function allocates and fills in a vector with pfns corresponding to
+ * virtual address range passed in arguments. If pfns have corresponding pages,
+ * page references are also grabbed to pin pages in memory. The function
+ * returns pointer to the vector on success and error pointer in case of
+ * failure. Returned vector needs to be freed via vb2_destroy_pfnvec().
+ */
+struct frame_vector *vb2_create_framevec(unsigned long start,
+unsigned long length,
+bool write)
+{
+ int ret;
+ unsigned long first, last;
+ unsigned long nr;
+ struct frame_vector *vec;
+
+ first = start >> PAGE_SHIFT;
+ last = (start + length - 1) >> PAGE_SHIFT;
+ nr = last - first + 1;
+ vec = frame_vector_create(nr);
+ if (!vec)
+ return ERR_PTR(-ENOMEM);
+ ret = get_vaddr_frames(start, nr, write, 1, vec);
+ if (ret < 0)
+ goto out_destroy;
+ /* We accept only complete set of PFNs */
+ if (ret != nr) {
+ ret = -EFAULT;
+ goto out_release;
+ }
+ return vec;
+out_release:
+ put_vaddr_frames(vec);
+out_destroy:
+ frame_vector_destroy(vec);
+ return ERR_PTR(ret);
+}
+EXPORT_SYMBOL(vb2_create_framevec);
+
+/**
+ * vb2_destroy_framevec() - release vector of mapped pfns
+ * @vec: vector of pfns / pages to release
+ *
+ * This releases references to all pages in the vector @vec (if corresponding
+ * pfns are backed by pages) and frees the passed vector.
+ */
+void vb2_destroy_framevec(struct frame_vector *vec)
+{
+ put_vaddr_frames(vec);
+ frame_vector_destroy(vec);
+}
+EXPORT_SYMBOL(vb2_destroy_framevec);
+
+/**
* vb2_common_vm_open() - increase refcount of the vma
* @vma: virtual memory region for the mapping
*
diff --git a/include/media/videobuf2-memops.h b/include/media/videobuf2-memops.h
index f05444ca8c0c..2f0564ff5f31 100644
--- a/include/media/videobuf2-memops.h
+++ b/include/media/videobuf2-memops.h
@@ -15,6 +15,7 @@
#define _MEDIA_VIDEOBUF2_MEMOPS_H
#include
+#include
/**
* vb2_vmarea_handler - common vma refcount tracking handler
@@ -36,5 +37,9 @@ int vb2_get_contig_userptr(unsigned long vaddr, unsigned long
size,
struct vm_area_struct *vb2_get_vma(struct vm_area_struct *vma);
void vb2_put_vma(struct vm_area_struct *vma);
+struct frame_vector *vb2_create_framevec(unsigned long start,
+unsigned long length,
+bool write);
+void vb2_destroy_framevec(struct frame_vector *vec);
#endif
--
2.1.4
[PATCH 3/9] media: omap_vout: Convert omap_vout_uservirt_to_phys() to use get_vaddr_pfns()
Convert omap_vout_uservirt_to_phys() to use get_vaddr_pfns() instead of
hand made mapping of virtual address to physical address. Also the
function leaked page reference from get_user_pages() so fix that by
properly release the reference when omap_vout_buffer_release() is
called.
Signed-off-by: Jan Kara
---
drivers/media/platform/omap/omap_vout.c | 67 +++--
1 file changed, 31 insertions(+), 36 deletions(-)
diff --git a/drivers/media/platform/omap/omap_vout.c
b/drivers/media/platform/omap/omap_vout.c
index 17b189a81ec5..d3f6d82ccbc1 100644
--- a/drivers/media/platform/omap/omap_vout.c
+++ b/drivers/media/platform/omap/omap_vout.c
@@ -195,46 +195,34 @@ static int omap_vout_try_format(struct v4l2_pix_format
*pix)
}
/*
- * omap_vout_uservirt_to_phys: This inline function is used to convert user
- * space virtual address to physical address.
+ * omap_vout_get_userptr: Convert user space virtual address to physical
+ * address.
*/
-static unsigned long omap_vout_uservirt_to_phys(unsigned long virtp)
+static int omap_vout_get_userptr(struct videobuf_buffer *vb, u32 virtp,
+u32 *physp)
{
- unsigned long physp = 0;
- struct vm_area_struct *vma;
- struct mm_struct *mm = current->mm;
+ struct frame_vector *vec;
+ int ret;
/* For kernel direct-mapped memory, take the easy way */
- if (virtp >= PAGE_OFFSET)
- return virt_to_phys((void *) virtp);
-
- down_read(¤t->mm->mmap_sem);
- vma = find_vma(mm, virtp);
- if (vma && (vma->vm_flags & VM_IO) && vma->vm_pgoff) {
- /* this will catch, kernel-allocated, mmaped-to-usermode
- addresses */
- physp = (vma->vm_pgoff << PAGE_SHIFT) + (virtp - vma->vm_start);
- up_read(¤t->mm->mmap_sem);
- } else {
- /* otherwise, use get_user_pages() for general userland pages */
- int res, nr_pages = 1;
- struct page *pages;
+ if (virtp >= PAGE_OFFSET) {
+ *physp = virt_to_phys((void *)virtp);
+ return 0;
+ }
- res = get_user_pages(current, current->mm, virtp, nr_pages, 1,
- 0, &pages, NULL);
- up_read(¤t->mm->mmap_sem);
+ vec = frame_vector_create(1);
+ if (!vec)
+ return -ENOMEM;
- if (res == nr_pages) {
- physp = __pa(page_address(&pages[0]) +
- (virtp & ~PAGE_MASK));
- } else {
- printk(KERN_WARNING VOUT_NAME
- "get_user_pages failed\n");
- return 0;
- }
+ ret = get_vaddr_frames(virtp, 1, 1, 0, vec);
+ if (ret != 1) {
+ frame_vector_destroy(vec);
+ return -EINVAL;
}
+ *physp = __pfn_to_phys(frame_vector_pfns(vec)[0]);
+ vb->priv = vec;
- return physp;
+ return 0;
}
/*
@@ -788,11 +776,15 @@ static int omap_vout_buffer_prepare(struct videobuf_queue
*q,
* address of the buffer
*/
if (V4L2_MEMORY_USERPTR == vb->memory) {
+ int ret;
+
if (0 == vb->baddr)
return -EINVAL;
/* Physical address */
- vout->queued_buf_addr[vb->i] = (u8 *)
- omap_vout_uservirt_to_phys(vb->baddr);
+ ret = omap_vout_get_userptr(vb, vb->baddr,
+ (u32 *)&vout->queued_buf_addr[vb->i]);
+ if (ret < 0)
+ return ret;
} else {
unsigned long addr, dma_addr;
unsigned long size;
@@ -841,9 +833,12 @@ static void omap_vout_buffer_release(struct videobuf_queue
*q,
struct omap_vout_device *vout = q->priv_data;
vb->state = VIDEOBUF_NEEDS_INIT;
+ if (vb->memory == V4L2_MEMORY_USERPTR && vb->priv) {
+ struct frame_vector *vec = vb->priv;
- if (V4L2_MEMORY_MMAP != vout->memory)
- return;
+ put_vaddr_frames(vec);
+ frame_vector_destroy(vec);
+ }
}
/*
--
2.1.4
[PATCH 2/9] mm: Provide new get_vaddr_frames() helper
Provide new function get_vaddr_frames(). This function maps virtual
addresses from given start and fills given array with page frame numbers of
the corresponding pages. If given start belongs to a normal vma, the function
grabs reference to each of the pages to pin them in memory. If start
belongs to VM_IO | VM_PFNMAP vma, we don't touch page structures. Caller
must make sure pfns aren't reused for anything else while he is using
them.
This function is created for various drivers to simplify handling of
their buffers.
Signed-off-by: Jan Kara
---
include/linux/mm.h | 44 +++
mm/gup.c | 214 +
2 files changed, 258 insertions(+)
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 0755b9fd03a7..dcd1f02a78e9 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -20,6 +20,7 @@
#include
#include
#include
+#include
struct mempolicy;
struct anon_vma;
@@ -1197,6 +1198,49 @@ long get_user_pages_unlocked(struct task_struct *tsk,
struct mm_struct *mm,
int write, int force, struct page **pages);
int get_user_pages_fast(unsigned long start, int nr_pages, int write,
struct page **pages);
+
+/* Container for pinned pfns / pages */
+struct frame_vector {
+ unsigned int nr_allocated; /* Number of frames we have space for */
+ unsigned int nr_frames; /* Number of frames stored in ptrs array */
+ bool got_ref; /* Did we pin pages by getting page ref? */
+ bool is_pfns; /* Does array contain pages or pfns? */
+ void *ptrs[0]; /* Array of pinned pfns / pages. Use
+* pfns_vector_pages() or pfns_vector_pfns()
+* for access */
+};
+
+struct frame_vector *frame_vector_create(unsigned int nr_frames);
+void frame_vector_destroy(struct frame_vector *vec);
+int get_vaddr_frames(unsigned long start, unsigned int nr_pfns,
+bool write, bool force, struct frame_vector *vec);
+void put_vaddr_frames(struct frame_vector *vec);
+int frame_vector_to_pages(struct frame_vector *vec);
+void frame_vector_to_pfns(struct frame_vector *vec);
+
+static inline unsigned int frame_vector_count(struct frame_vector *vec)
+{
+ return vec->nr_frames;
+}
+
+static inline struct page **frame_vector_pages(struct frame_vector *vec)
+{
+ if (vec->is_pfns) {
+ int err = frame_vector_to_pages(vec);
+
+ if (err)
+ return ERR_PTR(err);
+ }
+ return (struct page **)(vec->ptrs);
+}
+
+static inline unsigned long *frame_vector_pfns(struct frame_vector *vec)
+{
+ if (!vec->is_pfns)
+ frame_vector_to_pfns(vec);
+ return (unsigned long *)(vec->ptrs);
+}
+
struct kvec;
int get_kernel_pages(const struct kvec *iov, int nr_pages, int write,
struct page **pages);
diff --git a/mm/gup.c b/mm/gup.c
index 6297f6bccfb1..8db5c40e65c4 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -8,6 +8,7 @@
#include
#include
#include
+#include
#include
#include
@@ -936,6 +937,219 @@ int __mm_populate(unsigned long start, unsigned long len,
int ignore_errors)
return ret; /* 0 or negative error code */
}
+/*
+ * get_vaddr_frames() - map virtual addresses to pfns
+ * @start: starting user address
+ * @nr_frames: number of pages / pfns from start to map
+ * @write: whether pages will be written to by the caller
+ * @force: whether to force write access even if user mapping is
+ * readonly. This will result in the page being COWed even
+ * in MAP_SHARED mappings. You do not want this.
+ * @vec: structure which receives pages / pfns of the addresses mapped.
+ * It should have space for at least nr_frames entries.
+ *
+ * This function maps virtual addresses from @start and fills @vec structure
+ * with page frame numbers or page pointers to corresponding pages (choice
+ * depends on the type of the vma underlying the virtual address). If @start
+ * belongs to a normal vma, the function grabs reference to each of the pages
+ * to pin them in memory. If @start belongs to VM_IO | VM_PFNMAP vma, we don't
+ * touch page structures and the caller must make sure pfns aren't reused for
+ * anything else while he is using them.
+ *
+ * The function returns number of pages mapped which may be less than
+ * @nr_frames. In particular we stop mapping if there are more vmas of
+ * different type underlying the specified range of virtual addresses.
+ *
+ * This function takes care of grabbing mmap_sem as necessary.
+ */
+int get_vaddr_frames(unsigned long start, unsigned int nr_frames,
+bool write, bool force, struct frame_vector *vec)
+{
+ struct mm_struct *mm = current->mm;
+ struct vm_area_struct *vma;
+ int ret = 0;
+ int err;
+ int locked = 1;
+
+ if (nr_frames == 0)
+
[PATCH 1/9] [media] vb2: Push mmap_sem down to memops
Currently vb2 core acquires mmap_sem just around call to
__qbuf_userptr(). However since commit f035eb4e976ef5 (videobuf2: fix
lockdep warning) it isn't necessary to acquire it so early as we no
longer have to drop queue mutex before acquiring mmap_sem. So push
acquisition of mmap_sem down into .get_userptr and .put_userptr memops
so that the semaphore is acquired for a shorter time and it is clearer
what it is needed for.
Signed-off-by: Jan Kara
---
drivers/media/v4l2-core/videobuf2-core.c | 2 --
drivers/media/v4l2-core/videobuf2-dma-contig.c | 7 +++
drivers/media/v4l2-core/videobuf2-dma-sg.c | 6 ++
drivers/media/v4l2-core/videobuf2-vmalloc.c| 6 +-
4 files changed, 18 insertions(+), 3 deletions(-)
diff --git a/drivers/media/v4l2-core/videobuf2-core.c
b/drivers/media/v4l2-core/videobuf2-core.c
index 66ada01c796c..20cdbc0900ea 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -1657,9 +1657,7 @@ static int __buf_prepare(struct vb2_buffer *vb, const
struct v4l2_buffer *b)
ret = __qbuf_mmap(vb, b);
break;
case V4L2_MEMORY_USERPTR:
- down_read(¤t->mm->mmap_sem);
ret = __qbuf_userptr(vb, b);
- up_read(¤t->mm->mmap_sem);
break;
case V4L2_MEMORY_DMABUF:
ret = __qbuf_dmabuf(vb, b);
diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c
b/drivers/media/v4l2-core/videobuf2-dma-contig.c
index 644dec73d220..620c4aa78881 100644
--- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
@@ -532,7 +532,9 @@ static void vb2_dc_put_userptr(void *buf_priv)
sg_free_table(sgt);
kfree(sgt);
}
+ down_read(¤t->mm->mmap_sem);
vb2_put_vma(buf->vma);
+ up_read(¤t->mm->mmap_sem);
kfree(buf);
}
@@ -616,6 +618,7 @@ static void *vb2_dc_get_userptr(void *alloc_ctx, unsigned
long vaddr,
goto fail_buf;
}
+ down_read(¤t->mm->mmap_sem);
/* current->mm->mmap_sem is taken by videobuf2 core */
vma = find_vma(current->mm, vaddr);
if (!vma) {
@@ -642,6 +645,7 @@ static void *vb2_dc_get_userptr(void *alloc_ctx, unsigned
long vaddr,
if (ret) {
unsigned long pfn;
if (vb2_dc_get_user_pfn(start, n_pages, vma, &pfn) == 0) {
+ up_read(¤t->mm->mmap_sem);
buf->dma_addr = vb2_dc_pfn_to_dma(buf->dev, pfn);
buf->size = size;
kfree(pages);
@@ -651,6 +655,7 @@ static void *vb2_dc_get_userptr(void *alloc_ctx, unsigned
long vaddr,
pr_err("failed to get user pages\n");
goto fail_vma;
}
+ up_read(¤t->mm->mmap_sem);
sgt = kzalloc(sizeof(*sgt), GFP_KERNEL);
if (!sgt) {
@@ -713,10 +718,12 @@ fail_get_user_pages:
while (n_pages)
put_page(pages[--n_pages]);
+ down_read(¤t->mm->mmap_sem);
fail_vma:
vb2_put_vma(buf->vma);
fail_pages:
+ up_read(¤t->mm->mmap_sem);
kfree(pages); /* kfree is NULL-proof */
fail_buf:
diff --git a/drivers/media/v4l2-core/videobuf2-dma-sg.c
b/drivers/media/v4l2-core/videobuf2-dma-sg.c
index 45c708e463b9..afd4b514affc 100644
--- a/drivers/media/v4l2-core/videobuf2-dma-sg.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-sg.c
@@ -263,6 +263,7 @@ static void *vb2_dma_sg_get_userptr(void *alloc_ctx,
unsigned long vaddr,
if (!buf->pages)
goto userptr_fail_alloc_pages;
+ down_read(¤t->mm->mmap_sem);
vma = find_vma(current->mm, vaddr);
if (!vma) {
dprintk(1, "no vma for address %lu\n", vaddr);
@@ -301,6 +302,7 @@ static void *vb2_dma_sg_get_userptr(void *alloc_ctx,
unsigned long vaddr,
1, /* force */
buf->pages,
NULL);
+ up_read(¤t->mm->mmap_sem);
if (num_pages_from_user != buf->num_pages)
goto userptr_fail_get_user_pages;
@@ -328,8 +330,10 @@ userptr_fail_get_user_pages:
if (!vma_is_io(buf->vma))
while (--num_pages_from_user >= 0)
put_page(buf->pages[num_pages_from_user]);
+ down_read(¤t->mm->mmap_sem);
vb2_put_vma(buf->vma);
userptr_fail_find_vma:
+ up_read(¤t->mm->mmap_sem);
kfree(buf->pages);
userptr_fail_alloc_pages:
kfree(buf);
@@ -362,7 +366,9 @@ static void vb2_dma_sg_put_userptr(void *buf_priv)
put_page(buf->pages[i]);
}
kfree(buf->pages);
+ down_read(¤t->mm->mmap_sem);
vb2_put_vma(buf->vma);
+ up_read(¤t->mm->mmap_sem);
kfree(buf);
}
diff --git a/drivers/media/v4l2-core/videobuf2-vmalloc.c
b/drivers/media/v4l2-c
[PATCH 0/9 v4] Helper to abstract vma handling in media layer
Hello, Sorry for a quick resend but the missing vmalloc include in mm/gup.c is annoying for archs that need it and I was also told to extend CC list a bit. I'm sending the fourth version of my patch series to abstract vma handling from the various media drivers. After this patch set drivers have to know much less details about vmas, their types, and locking. Also quite some code is removed from them. As a bonus drivers get automatically VM_FAULT_RETRY handling. The primary motivation for this series is to remove knowledge about mmap_sem locking from as many places a possible so that we can change it with reasonable effort. The core of the series is the new helper get_vaddr_frames() which is given a virtual address and it fills in PFNs / struct page pointers (depending on VMA type) into the provided array. If PFNs correspond to normal pages it also grabs references to these pages. The difference from get_user_pages() is that this function can also deal with pfnmap, and io mappings which is what the media drivers need. I have tested the patches with vivid driver so at least vb2 code got some exposure. Conversion of other drivers was just compile-tested (for x86 so e.g. exynos driver which is only for Samsung platform is completely untested) so I'd like to ask respective maintainers if they could have a look. Also I'd like to ask mm folks to check patch 2/9 implementing the helper. Thanks! Honza Changes since v3: * Added include into mm/gup.c as it's needed for some archs * Fixed error path for exynos driver Changes since v2: * Renamed functions and structures as Mel suggested * Other minor changes suggested by Mel * Rebased on top of 4.1-rc2 * Changed functions to get pointer to array of pages / pfns to perform conversion if necessary. This fixes possible issue in the omap I may have introduced in v2 and generally makes the API less errorprone.
[PATCH] drm/msm: Fix compil issue when DRM_MSM_FBDEV is disabled
When CONFIG_DRM_MSM_FBDEV is not defined,
CONFIG_DRM_KMS_FB_HELPER does not get selected and
drm_fb_helper_*() helper functions are thus not available.
This change fixes these link issues.
Signed-off-by: Stephane Viau
---
drivers/gpu/drm/msm/msm_drv.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 2b1218c..35380ec 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -21,9 +21,11 @@
static void msm_fb_output_poll_changed(struct drm_device *dev)
{
+#ifdef DRM_MSM_FBDEV
struct msm_drm_private *priv = dev->dev_private;
if (priv->fbdev)
drm_fb_helper_hotplug_event(priv->fbdev);
+#endif
}
static const struct drm_mode_config_funcs mode_config_funcs = {
@@ -419,9 +421,11 @@ static void msm_preclose(struct drm_device *dev, struct
drm_file *file)
static void msm_lastclose(struct drm_device *dev)
{
+#ifdef DRM_MSM_FBDEV
struct msm_drm_private *priv = dev->dev_private;
if (priv->fbdev)
drm_fb_helper_restore_fbdev_mode_unlocked(priv->fbdev);
+#endif
}
static irqreturn_t msm_irq(int irq, void *arg)
--
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a
Linux Foundation Collaborative Project
[Bug 67681] Asus G75VW F keys Not Working For WiFi & Brightness
https://bugzilla.kernel.org/show_bug.cgi?id=67681 --- Comment #40 from KernelBug <3fdd1e5d at opayq.com> --- I just noticed in the kernel under; Graphics support > Frame buffer Devices > CONFIG_FB_NVIDIA_BACKLIGHT: This is the description; Say Y here if you want to control the backlight of your display. If this is what I'm looking at and reading, for the backlight of the monitor to control the brightness, WHY is this under the Frame Buffer section, and not placed under, 'Backlight & LCD device support'? I'm really confused by this and just don't understand... Can someone PLEASE explain this? Because I have been waiting since Dec. 2013, hoping that the Kernel Dev team will add in more backlight support, only to find under the Framebuffer, which trying to make Nvidia Framebuffer work is a pain in the ass... -- You are receiving this mail because: You are watching the assignee of the bug.
[RFC] How implement Secure Data Path ?
On Wed, May 6, 2015 at 4:35 AM, Daniel Vetter wrote:
> On Tue, May 05, 2015 at 05:54:05PM +0100, One Thousand Gnomes wrote:
>> > First what is Secure Data Path ? SDP is a set of hardware features to
>> > garanty
>> > that some memories regions could only be read and/or write by specific
>> > hardware
>> > IPs. You can imagine it as a kind of memory firewall which grant/revoke
>> > accesses to memory per devices. Firewall configuration must be done in a
>> > trusted
>> > environment: for ARM architecture we plan to use OP-TEE + a trusted
>> > application to do that.
>>
>> It's not just an ARM feature so any basis for this in the core code
>> should be generic, whether its being enforced by ARM SDP, various Intel
>> feature sets or even via a hypervisor.
>>
>> > I have try 2 "hacky" approachs with dma_buf:
>> > - add a secure field in dma_buf structure and configure firewall in
>> > dma_buf_{map/unmap}_attachment() functions.
>>
>> How is SDP not just another IOMMU. The only oddity here is that it
>> happens to configure buffers the CPU can't touch and it has a control
>> mechanism that is designed to cover big media corp type uses where the
>> threat model is that the system owner is the enemy. Why does anything care
>> about it being SDP, there are also generic cases this might be a useful
>> optimisation (eg knowing the buffer isn't CPU touched so you can optimise
>> cache flushing).
>>
>> The control mechanism is a device/platform detail as with any IOMMU. It
>> doesn't matter who configures it or how, providing it happens.
>>
>> We do presumably need some small core DMA changes - anyone trying to map
>> such a buffer into CPU space needs to get a warning or error but what
>> else ?
>>
>> > >From buffer allocation point of view I also facing a problem because when
>> > >v4l2
>> > or drm/kms are exporting buffers by using dma_buf they don't attaching
>> > themself on it and never call dma_buf_{map/unmap}_attachment(). This is not
>> > an issue in those framework while it is how dma_buf exporters are
>> > supposed to work.
>>
>> Which could be addressed if need be.
>>
>> So if "SDP" is just another IOMMU feature, just as stuff like IMR is on
>> some x86 devices, and hypervisor enforced protection is on assorted
>> platforms why do we need a special way to do it ? Is there anything
>> actually needed beyond being able to tell the existing DMA code that this
>> buffer won't be CPU touched and wiring it into the DMA operations for the
>> platform ?
>
> Iirc most of the dma api stuff gets unhappy when memory isn't struct page
> backed. In i915 we do use sg tables everywhere though (even for memory not
> backed by struct page, e.g. the "stolen" range the bios prereserves), but
> we fill those out manually.
>
> A possible generic design I see is to have a secure memory allocator
> device which doesn nothing else but hand out dma-bufs. With that we can
> hide the platform-specific allocation methods in there (some need to
> allocate from carveouts, other just need to mark the pages specifically).
> Also dma-buf has explicit methods for cpu access, which are allowed to
> fail. And using the dma-buf attach tracking we can also reject dma to
> devices which cannot access the secure memory. Given all that I think
> going through the dma-buf interface but with a special-purpose allocator
> seems to fit.
>
> I'm not sure whether a special iommu is a good idea otoh: I'd expect that
> for most devices the driver would need to decide about which iommu to pick
> (or maybe keep track of some special flags for an extended dma_map
> interface). At least looking at gpu drivers using iommus would require
> special code, whereas fully hiding all this behind the dma-buf interface
> should fit in much better.
jfwiw, I'd fully expect devices to be dealing with a mix of secure and
insecure buffers, so I'm also not really sure how the 'special iommu'
plan would play out..
I think 'secure' allocator device sounds attractive from PoV of
separating out platform nonsense.. not sure if it is exactly that
easy, since importing device probably needs to set some special bits
here and there..
BR,
-R
> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
No HDMI Audio with Radeon HD7750 on Acube Sam460ex AMCC powerpc 460ex board
00 >> [4.469662] [drm] vram apper at 0x8000 >> [4.473775] [drm] size 8294400 >> [4.476840] [drm] fb depth is 24 >> [4.480084] [drm]pitch is 7680 >> [4.759376] Console: switching to colour frame buffer device 240x67 >> [4.838736] radeon 0001:81:00.0: fb0: radeondrmfb frame buffer device >> [4.845560] radeon 0001:81:00.0: registered panic notifier >> [4.856007] [drm] Initialized radeon 2.40.0 20080528 for 0001:81:00.0 on >> minor 0 >> >> [ 31.335598] [drm:dce6_audio_get_pin] *ERROR* No connected audio pins >> found! >> >> >> >> ___ >> dri-devel mailing list >> dri-devel at lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/dri-devel >> -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150506/9f0ff255/attachment-0001.html>
[Bug 90320] Lenovo ThinkPad E455 (Kaveri A10-7300): Blank built-in screen with radeon kms driver
https://bugs.freedesktop.org/show_bug.cgi?id=90320 --- Comment #5 from Samir IbradžiÄ --- Created attachment 115573 --> https://bugs.freedesktop.org/attachment.cgi?id=115573&action=edit dmesg with drm.debug=15 Attached dmesg with drm.debug=15, is shows seemingly correct eDP link training -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150506/29a6f65b/attachment.html>
[Bug 90320] Lenovo ThinkPad E455 (Kaveri A10-7300): Blank built-in screen with radeon kms driver
https://bugs.freedesktop.org/show_bug.cgi?id=90320 --- Comment #4 from Samir IbradžiÄ --- Attached xorg logs, both for radeon kms & fglrx. Anyone knows if fglrx has some kind of debug log where DP link training bits may be visible? -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150506/583643ce/attachment.html>
[Bug 90320] Lenovo ThinkPad E455 (Kaveri A10-7300): Blank built-in screen with radeon kms driver
https://bugs.freedesktop.org/show_bug.cgi?id=90320 --- Comment #3 from Samir IbradžiÄ --- Created attachment 115571 --> https://bugs.freedesktop.org/attachment.cgi?id=115571&action=edit Xorg.0.log fglrx -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150506/029935eb/attachment.html>
[Bug 90320] Lenovo ThinkPad E455 (Kaveri A10-7300): Blank built-in screen with radeon kms driver
https://bugs.freedesktop.org/show_bug.cgi?id=90320 --- Comment #2 from Samir IbradžiÄ --- Created attachment 115570 --> https://bugs.freedesktop.org/attachment.cgi?id=115570&action=edit Xorg.0.log radeon kms -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150506/d7240438/attachment.html>
[RFC] How implement Secure Data Path ?
On Tuesday 05 May 2015 09:27:52 Christoph Hellwig wrote: > On Tue, May 05, 2015 at 05:39:57PM +0200, Benjamin Gaignard wrote: > > Since few months I'm looking for Linaro to how do Secure Data Path (SPD). > > I have tried and implemented multiple thinks but I always facing > > architecture issues so I would like to get your help to solve the > > problem. > > > > First what is Secure Data Path ? SDP is a set of hardware features to > > garanty that some memories regions could only be read and/or write by > > specific hardware IPs. You can imagine it as a kind of memory firewall > > which grant/revoke accesses to memory per devices. Firewall configuration > > must be done in a trusted environment: for ARM architecture we plan to > > use OP-TEE + a trusted application to do that. > > > > One typical use case for SDP in a video playback which involve those > > elements: decrypt -> video decoder -> transform -> display > > Sounds like a good enough reason not to implement it ever. The irony of it is to post an RFC on they day before http://www.defectivebydesign.org/dayagainstdrm/ :-) -- Regards, Laurent Pinchart
