From 2678aeba5fe1a799788cce2a34003d8fa6ad7153 Mon Sep 17 00:00:00 2001 From: Thierry Reding Date: Mon, 18 Mar 2013 11:09:13 +0100 Subject: [PATCH 01/16] drm/tegra: Don't disable unused planes When a plane isn't in use it isn't attached to a CRTC and therefore the DC registers aren't available for programming. Signed-off-by: Thierry Reding --- drivers/gpu/host1x/drm/dc.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/host1x/drm/dc.c b/drivers/gpu/host1x/drm/dc.c index 8c04943f82e3..98f60d827558 100644 --- a/drivers/gpu/host1x/drm/dc.c +++ b/drivers/gpu/host1x/drm/dc.c @@ -79,6 +79,9 @@ static int tegra_plane_disable(struct drm_plane *plane) struct tegra_plane *p = to_tegra_plane(plane); unsigned long value; + if (!plane->crtc) + return 0; + value = WINDOW_A_SELECT << p->index; tegra_dc_writel(dc, value, DC_CMD_DISPLAY_WINDOW_HEADER); From 603f0cc9482e5e5996b84d3ffb5578526974809c Mon Sep 17 00:00:00 2001 From: Thierry Reding Date: Mon, 22 Apr 2013 21:22:14 +0200 Subject: [PATCH 02/16] drm/tegra: Explicitly set irq_enabled Since the Tegra DRM driver doesn't use the drm_irq_install() helper, the irq_enabled flag needs to be set manually in order to make functionality such as the DRM_IOCTL_WAIT_VBLANK work properly. Signed-off-by: Thierry Reding --- drivers/gpu/host1x/drm/drm.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/drivers/gpu/host1x/drm/drm.c b/drivers/gpu/host1x/drm/drm.c index 2b561c9118c6..78b07db37cf8 100644 --- a/drivers/gpu/host1x/drm/drm.c +++ b/drivers/gpu/host1x/drm/drm.c @@ -257,6 +257,13 @@ static int tegra_drm_load(struct drm_device *drm, unsigned long flags) if (err < 0) return err; + /* + * We don't use the drm_irq_install() helpers provided by the DRM + * core, so we need to set this manually in order to allow the + * DRM_IOCTL_WAIT_VBLANK to operate correctly. + */ + drm->irq_enabled = 1; + err = drm_vblank_init(drm, drm->mode_config.num_crtc); if (err < 0) return err; From ed683aead1962f2242c8dc8863517bcd3089703d Mon Sep 17 00:00:00 2001 From: Thierry Reding Date: Mon, 22 Apr 2013 21:31:15 +0200 Subject: [PATCH 03/16] drm/tegra: Honor pixel-format changes When using a base mode-set, honor changes in pixel-format since the core doesn't explicitly check for them as long as they use the same depth. Signed-off-by: Thierry Reding --- drivers/gpu/host1x/drm/dc.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/host1x/drm/dc.c b/drivers/gpu/host1x/drm/dc.c index 98f60d827558..5360e5a57ecc 100644 --- a/drivers/gpu/host1x/drm/dc.c +++ b/drivers/gpu/host1x/drm/dc.c @@ -143,6 +143,7 @@ static int tegra_dc_add_planes(struct drm_device *drm, struct tegra_dc *dc) static int tegra_dc_set_base(struct tegra_dc *dc, int x, int y, struct drm_framebuffer *fb) { + unsigned int format = tegra_dc_format(fb->pixel_format); struct tegra_bo *bo = tegra_fb_get_plane(fb, 0); unsigned long value; @@ -153,6 +154,7 @@ static int tegra_dc_set_base(struct tegra_dc *dc, int x, int y, tegra_dc_writel(dc, bo->paddr + value, DC_WINBUF_START_ADDR); tegra_dc_writel(dc, fb->pitches[0], DC_WIN_LINE_STRIDE); + tegra_dc_writel(dc, format, DC_WIN_COLOR_DEPTH); value = GENERAL_UPDATE | WIN_A_UPDATE; tegra_dc_writel(dc, value, DC_CMD_STATE_CONTROL); From b6f2056f3b259960b0cb6a6cf440b89f2567d586 Mon Sep 17 00:00:00 2001 From: Wei Yongjun Date: Wed, 24 Apr 2013 10:48:23 +0800 Subject: [PATCH 04/16] drm/tegra: fix missing unlock on error Add the missing unlock before return from function host1x_drm_init() and host1x_drm_exit() in the error handling case. Signed-off-by: Wei Yongjun Signed-off-by: Thierry Reding --- drivers/gpu/host1x/drm/drm.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/host1x/drm/drm.c b/drivers/gpu/host1x/drm/drm.c index 78b07db37cf8..66629f398f11 100644 --- a/drivers/gpu/host1x/drm/drm.c +++ b/drivers/gpu/host1x/drm/drm.c @@ -148,6 +148,7 @@ int host1x_drm_init(struct host1x_drm *host1x, struct drm_device *drm) dev_err(host1x->dev, "DRM setup failed for %s: %d\n", dev_name(client->dev), err); + mutex_unlock(&host1x->clients_lock); return err; } } @@ -175,6 +176,7 @@ int host1x_drm_exit(struct host1x_drm *host1x) dev_err(host1x->dev, "DRM cleanup failed for %s: %d\n", dev_name(client->dev), err); + mutex_unlock(&host1x->clients_lock); return err; } } From 2639f2bf68e4c1bafafc0feca3e16cdad2333a82 Mon Sep 17 00:00:00 2001 From: Wei Yongjun Date: Wed, 24 Apr 2013 14:50:51 +0800 Subject: [PATCH 05/16] drm/tegra: fix error return code in gr2d_submit() Fix to return -ENOENT in the host1x_bo lookup error handling case instead of 0, as done elsewhere in this function. Signed-off-by: Wei Yongjun Signed-off-by: Thierry Reding --- drivers/gpu/host1x/drm/gr2d.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/host1x/drm/gr2d.c b/drivers/gpu/host1x/drm/gr2d.c index 6a45ae090ee7..aca72fc5e2a2 100644 --- a/drivers/gpu/host1x/drm/gr2d.c +++ b/drivers/gpu/host1x/drm/gr2d.c @@ -135,8 +135,10 @@ static int gr2d_submit(struct host1x_drm_context *context, goto fail; bo = host1x_bo_lookup(drm, file, cmdbuf.handle); - if (!bo) + if (!bo) { + err = -ENOENT; goto fail; + } host1x_job_add_gather(job, bo, cmdbuf.words, cmdbuf.offset); num_cmdbufs--; @@ -158,8 +160,10 @@ static int gr2d_submit(struct host1x_drm_context *context, reloc->cmdbuf = cmdbuf; reloc->target = target; - if (!reloc->target || !reloc->cmdbuf) + if (!reloc->target || !reloc->cmdbuf) { + err = -ENOENT; goto fail; + } } err = copy_from_user(job->waitchk, waitchks, From a5ad7a636b2b5925aaad6430a258ed523cef8d96 Mon Sep 17 00:00:00 2001 From: Thierry Reding Date: Sun, 19 May 2013 14:19:21 +0200 Subject: [PATCH 06/16] MAINTAINERS: Update Tegra DRM entry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Change email to my private address, point file entry to the new location below drivers/gpu/host1x, add include/uapi/drm/tegra_drm.h to maintained file list and update git tree. Furthermore, Terje Bergström will be co-maintaining. Signed-off-by: Thierry Reding --- MAINTAINERS | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index 829c0321108b..4f7d0bac1432 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2697,12 +2697,14 @@ F: include/drm/exynos* F: include/uapi/drm/exynos* DRM DRIVERS FOR NVIDIA TEGRA -M: Thierry Reding +M: Thierry Reding +M: Terje Bergström L: dri-devel@lists.freedesktop.org L: linux-tegra@vger.kernel.org -T: git git://gitorious.org/thierryreding/linux.git +T: git git://anongit.freedesktop.org/tegra/linux.git S: Maintained -F: drivers/gpu/drm/tegra/ +F: drivers/gpu/host1x/ +F: include/uapi/drm/tegra_drm.h F: Documentation/devicetree/bindings/gpu/nvidia,tegra20-host1x.txt DSBR100 USB FM RADIO DRIVER From a191e48d442f2d996e2a7292802a2ad22ecb503a Mon Sep 17 00:00:00 2001 From: Emil Goode Date: Fri, 26 Apr 2013 19:49:51 +0200 Subject: [PATCH 07/16] drm/tegra: Include header drm/drm.h Include definitions of used types by including drm/drm.h Sparse output: /usr/include/drm/tegra_drm.h:21: found __[us]{8,16,32,64} type without #include Signed-off-by: Emil Goode Signed-off-by: Thierry Reding --- include/uapi/drm/tegra_drm.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/uapi/drm/tegra_drm.h b/include/uapi/drm/tegra_drm.h index 6e132a2f7420..73bde4eaf16c 100644 --- a/include/uapi/drm/tegra_drm.h +++ b/include/uapi/drm/tegra_drm.h @@ -17,6 +17,8 @@ #ifndef _UAPI_TEGRA_DRM_H_ #define _UAPI_TEGRA_DRM_H_ +#include + struct drm_tegra_gem_create { __u64 size; __u32 flags; From dc618db75df9b930293786b4dc763fb5ea7bed15 Mon Sep 17 00:00:00 2001 From: Thierry Reding Date: Sun, 19 May 2013 14:21:22 +0200 Subject: [PATCH 08/16] drm/tegra: Fix return value Return NULL instead of 0 in host1x_bo_lookup(). Signed-off-by: Thierry Reding --- drivers/gpu/host1x/drm/gr2d.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/host1x/drm/gr2d.c b/drivers/gpu/host1x/drm/gr2d.c index aca72fc5e2a2..7efd97b2aaa4 100644 --- a/drivers/gpu/host1x/drm/gr2d.c +++ b/drivers/gpu/host1x/drm/gr2d.c @@ -84,7 +84,7 @@ static struct host1x_bo *host1x_bo_lookup(struct drm_device *drm, gem = drm_gem_object_lookup(drm, file, handle); if (!gem) - return 0; + return NULL; mutex_lock(&drm->struct_mutex); drm_gem_object_unreference(gem); From 604faa7dcf772ea4d13f89f0c3cc642b15cc4f45 Mon Sep 17 00:00:00 2001 From: Laurent Pinchart Date: Wed, 29 May 2013 07:44:34 +0200 Subject: [PATCH 09/16] drm/tegra: Remove DRIVER_BUS_PLATFORM from driver_features DRIVER_BUS_PLATFORM is not a DRM driver feature flag, it must not be set in the driver's driver_features field. Signed-off-by: Laurent Pinchart Signed-off-by: Thierry Reding --- drivers/gpu/host1x/drm/drm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/host1x/drm/drm.c b/drivers/gpu/host1x/drm/drm.c index 66629f398f11..c171a07f47bf 100644 --- a/drivers/gpu/host1x/drm/drm.c +++ b/drivers/gpu/host1x/drm/drm.c @@ -614,7 +614,7 @@ static void tegra_debugfs_cleanup(struct drm_minor *minor) #endif struct drm_driver tegra_drm_driver = { - .driver_features = DRIVER_BUS_PLATFORM | DRIVER_MODESET | DRIVER_GEM, + .driver_features = DRIVER_MODESET | DRIVER_GEM, .load = tegra_drm_load, .unload = tegra_drm_unload, .open = tegra_drm_open, From 64c173d3a2bb367c901f1f0a66c1d4e338d8cb2c Mon Sep 17 00:00:00 2001 From: Terje Bergstrom Date: Wed, 29 May 2013 13:26:02 +0300 Subject: [PATCH 10/16] gpu: host1x: Check INCR opcode correctly The firewall code used a wrong loop condition (pointer to a structure) while checking INCR opcode. This patch fixes the code to use correct loop condition (number of words remaining). Signed-off-by: Terje Bergstrom Signed-off-by: Arto Merilainen Signed-off-by: Thierry Reding --- drivers/gpu/host1x/job.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/host1x/job.c b/drivers/gpu/host1x/job.c index f665d679031c..2974ac8d70a7 100644 --- a/drivers/gpu/host1x/job.c +++ b/drivers/gpu/host1x/job.c @@ -330,7 +330,7 @@ static int check_incr(struct host1x_firewall *fw) u32 count = fw->count; u32 reg = fw->reg; - while (fw) { + while (count) { if (fw->words == 0) return -EINVAL; From 5060d8ec7cfc29dd399b4fe952ba96e7a88aa778 Mon Sep 17 00:00:00 2001 From: Arto Merilainen Date: Wed, 29 May 2013 13:26:03 +0300 Subject: [PATCH 11/16] gpu: host1x: Check reloc table before usage The firewall assumed that the user space always delivers a relocation table when it is accessing address registers. If userspace did not deliver a relocation table and tried to access the address registers, the code performed bad memory accesses. This patch modifies the firewall to check correctly that the firewall table is available before accessing it. In addition, check_reloc() is converted to use boolean return value (true when the reloc is valid, false when invalid). Signed-off-by: Arto Merilainen Acked-By: Terje Bergstrom Signed-off-by: Thierry Reding --- drivers/gpu/host1x/job.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/host1x/job.c b/drivers/gpu/host1x/job.c index 2974ac8d70a7..83804fdf9c99 100644 --- a/drivers/gpu/host1x/job.c +++ b/drivers/gpu/host1x/job.c @@ -268,15 +268,15 @@ static unsigned int do_relocs(struct host1x_job *job, struct host1x_bo *cmdbuf) return 0; } -static int check_reloc(struct host1x_reloc *reloc, struct host1x_bo *cmdbuf, +static bool check_reloc(struct host1x_reloc *reloc, struct host1x_bo *cmdbuf, unsigned int offset) { offset *= sizeof(u32); if (reloc->cmdbuf != cmdbuf || reloc->cmdbuf_offset != offset) - return -EINVAL; + return false; - return 0; + return true; } struct host1x_firewall { @@ -307,10 +307,10 @@ static int check_mask(struct host1x_firewall *fw) if (mask & 1) { if (fw->job->is_addr_reg(fw->dev, fw->class, reg)) { - bool bad_reloc = check_reloc(fw->reloc, - fw->cmdbuf_id, - fw->offset); - if (!fw->num_relocs || bad_reloc) + if (!fw->num_relocs) + return -EINVAL; + if (!check_reloc(fw->reloc, fw->cmdbuf_id, + fw->offset)) return -EINVAL; fw->reloc++; fw->num_relocs--; @@ -335,9 +335,9 @@ static int check_incr(struct host1x_firewall *fw) return -EINVAL; if (fw->job->is_addr_reg(fw->dev, fw->class, reg)) { - bool bad_reloc = check_reloc(fw->reloc, fw->cmdbuf_id, - fw->offset); - if (!fw->num_relocs || bad_reloc) + if (!fw->num_relocs) + return -EINVAL; + if (!check_reloc(fw->reloc, fw->cmdbuf_id, fw->offset)) return -EINVAL; fw->reloc++; fw->num_relocs--; @@ -361,9 +361,9 @@ static int check_nonincr(struct host1x_firewall *fw) return -EINVAL; if (is_addr_reg) { - bool bad_reloc = check_reloc(fw->reloc, fw->cmdbuf_id, - fw->offset); - if (!fw->num_relocs || bad_reloc) + if (!fw->num_relocs) + return -EINVAL; + if (!check_reloc(fw->reloc, fw->cmdbuf_id, fw->offset)) return -EINVAL; fw->reloc++; fw->num_relocs--; From afac0e43c6c98473cce18fdeb5f7dda86dcf244f Mon Sep 17 00:00:00 2001 From: Terje Bergstrom Date: Wed, 29 May 2013 13:26:04 +0300 Subject: [PATCH 12/16] gpu: host1x: Don't reset firewall between gathers The firewall was reinitialised for each gather. Because the filter was reinitialised, it did not track the class over gather boundaries. This allowed the user application to set host1x class to one class in one gather and use that class in another gather without firewall having knowledge about that. Signed-off-by: Terje Bergstrom Signed-off-by: Arto Merilainen Signed-off-by: Thierry Reding --- drivers/gpu/host1x/job.c | 72 +++++++++++++++++++--------------------- 1 file changed, 34 insertions(+), 38 deletions(-) diff --git a/drivers/gpu/host1x/job.c b/drivers/gpu/host1x/job.c index 83804fdf9c99..5b9548f610f1 100644 --- a/drivers/gpu/host1x/job.c +++ b/drivers/gpu/host1x/job.c @@ -376,69 +376,60 @@ static int check_nonincr(struct host1x_firewall *fw) return 0; } -static int validate(struct host1x_job *job, struct device *dev, - struct host1x_job_gather *g) +static int validate(struct host1x_firewall *fw, struct host1x_job_gather *g) { u32 *cmdbuf_base; int err = 0; - struct host1x_firewall fw; - fw.job = job; - fw.dev = dev; - fw.reloc = job->relocarray; - fw.num_relocs = job->num_relocs; - fw.cmdbuf_id = g->bo; - - fw.offset = 0; - fw.class = 0; - - if (!job->is_addr_reg) + if (!fw->job->is_addr_reg) return 0; cmdbuf_base = host1x_bo_mmap(g->bo); if (!cmdbuf_base) return -ENOMEM; + fw->words = g->words; + fw->cmdbuf_id = g->bo; + fw->offset = 0; - fw.words = g->words; - while (fw.words && !err) { - u32 word = cmdbuf_base[fw.offset]; + while (fw->words && !err) { + u32 word = cmdbuf_base[fw->offset]; u32 opcode = (word & 0xf0000000) >> 28; - fw.mask = 0; - fw.reg = 0; - fw.count = 0; - fw.words--; - fw.offset++; + fw->mask = 0; + fw->reg = 0; + fw->count = 0; + fw->words--; + fw->offset++; switch (opcode) { case 0: - fw.class = word >> 6 & 0x3ff; - fw.mask = word & 0x3f; - fw.reg = word >> 16 & 0xfff; - err = check_mask(&fw); + fw->class = word >> 6 & 0x3ff; + fw->mask = word & 0x3f; + fw->reg = word >> 16 & 0xfff; + err = check_mask(fw); if (err) goto out; break; case 1: - fw.reg = word >> 16 & 0xfff; - fw.count = word & 0xffff; - err = check_incr(&fw); + fw->reg = word >> 16 & 0xfff; + fw->count = word & 0xffff; + err = check_incr(fw); if (err) goto out; break; case 2: - fw.reg = word >> 16 & 0xfff; - fw.count = word & 0xffff; - err = check_nonincr(&fw); + fw->reg = word >> 16 & 0xfff; + fw->count = word & 0xffff; + err = check_nonincr(fw); if (err) goto out; break; case 3: - fw.mask = word & 0xffff; - fw.reg = word >> 16 & 0xfff; - err = check_mask(&fw); + fw->mask = word & 0xffff; + fw->reg = word >> 16 & 0xfff; + err = check_mask(fw); if (err) goto out; break; @@ -453,12 +444,10 @@ static int validate(struct host1x_job *job, struct device *dev, } /* No relocs should remain at this point */ - if (fw.num_relocs) + if (fw->num_relocs) err = -EINVAL; out: - host1x_bo_munmap(g->bo, cmdbuf_base); - return err; } @@ -508,8 +497,15 @@ int host1x_job_pin(struct host1x_job *job, struct device *dev) int err; unsigned int i, j; struct host1x *host = dev_get_drvdata(dev->parent); + struct host1x_firewall fw; DECLARE_BITMAP(waitchk_mask, host1x_syncpt_nb_pts(host)); + fw.job = job; + fw.dev = dev; + fw.reloc = job->relocarray; + fw.num_relocs = job->num_relocs; + fw.class = 0; + bitmap_zero(waitchk_mask, host1x_syncpt_nb_pts(host)); for (i = 0; i < job->num_waitchk; i++) { u32 syncpt_id = job->waitchk[i].syncpt_id; @@ -543,7 +539,7 @@ int host1x_job_pin(struct host1x_job *job, struct device *dev) err = 0; if (IS_ENABLED(CONFIG_TEGRA_HOST1X_FIREWALL)) - err = validate(job, dev, g); + err = validate(&fw, g); if (err) dev_err(dev, "Job invalid (err=%d)\n", err); From 3364cd28906d87f0c77754998679bb66639d4112 Mon Sep 17 00:00:00 2001 From: Arto Merilainen Date: Wed, 29 May 2013 13:26:05 +0300 Subject: [PATCH 13/16] gpu: host1x: Copy gathers before verification The firewall verified gather buffers before copying them. This allowed a malicious application to rewrite the buffer content by timing the rewrite carefully. This patch makes the buffer validation occur after copying the buffers. Signed-off-by: Arto Merilainen Signed-off-by: Terje Bergstrom Signed-off-by: Thierry Reding --- drivers/gpu/host1x/job.c | 51 ++++++++++++++++------------------------ 1 file changed, 20 insertions(+), 31 deletions(-) diff --git a/drivers/gpu/host1x/job.c b/drivers/gpu/host1x/job.c index 5b9548f610f1..cc807667d8f1 100644 --- a/drivers/gpu/host1x/job.c +++ b/drivers/gpu/host1x/job.c @@ -228,17 +228,15 @@ static unsigned int do_relocs(struct host1x_job *job, struct host1x_bo *cmdbuf) void *cmdbuf_page_addr = NULL; /* pin & patch the relocs for one gather */ - while (i < job->num_relocs) { + for (i = 0; i < job->num_relocs; i++) { struct host1x_reloc *reloc = &job->relocarray[i]; u32 reloc_addr = (job->reloc_addr_phys[i] + reloc->target_offset) >> reloc->shift; u32 *target; /* skip all other gathers */ - if (!(reloc->cmdbuf && cmdbuf == reloc->cmdbuf)) { - i++; + if (cmdbuf != reloc->cmdbuf) continue; - } if (last_page != reloc->cmdbuf_offset >> PAGE_SHIFT) { if (cmdbuf_page_addr) @@ -257,9 +255,6 @@ static unsigned int do_relocs(struct host1x_job *job, struct host1x_bo *cmdbuf) target = cmdbuf_page_addr + (reloc->cmdbuf_offset & ~PAGE_MASK); *target = reloc_addr; - - /* mark this gather as handled */ - reloc->cmdbuf = 0; } if (cmdbuf_page_addr) @@ -378,15 +373,13 @@ static int check_nonincr(struct host1x_firewall *fw) static int validate(struct host1x_firewall *fw, struct host1x_job_gather *g) { - u32 *cmdbuf_base; + u32 *cmdbuf_base = (u32 *)fw->job->gather_copy_mapped + + (g->offset / sizeof(u32)); int err = 0; if (!fw->job->is_addr_reg) return 0; - cmdbuf_base = host1x_bo_mmap(g->bo); - if (!cmdbuf_base) - return -ENOMEM; fw->words = g->words; fw->cmdbuf_id = g->bo; fw->offset = 0; @@ -453,10 +446,17 @@ out: static inline int copy_gathers(struct host1x_job *job, struct device *dev) { + struct host1x_firewall fw; size_t size = 0; size_t offset = 0; int i; + fw.job = job; + fw.dev = dev; + fw.reloc = job->relocarray; + fw.num_relocs = job->num_relocs; + fw.class = 0; + for (i = 0; i < job->num_gathers; i++) { struct host1x_job_gather *g = &job->gathers[i]; size += g->words * sizeof(u32); @@ -477,14 +477,19 @@ static inline int copy_gathers(struct host1x_job *job, struct device *dev) struct host1x_job_gather *g = &job->gathers[i]; void *gather; + /* Copy the gather */ gather = host1x_bo_mmap(g->bo); memcpy(job->gather_copy_mapped + offset, gather + g->offset, g->words * sizeof(u32)); host1x_bo_munmap(g->bo, gather); + /* Store the location in the buffer */ g->base = job->gather_copy; g->offset = offset; - g->bo = NULL; + + /* Validate the job */ + if (validate(&fw, g)) + return -EINVAL; offset += g->words * sizeof(u32); } @@ -497,15 +502,8 @@ int host1x_job_pin(struct host1x_job *job, struct device *dev) int err; unsigned int i, j; struct host1x *host = dev_get_drvdata(dev->parent); - struct host1x_firewall fw; DECLARE_BITMAP(waitchk_mask, host1x_syncpt_nb_pts(host)); - fw.job = job; - fw.dev = dev; - fw.reloc = job->relocarray; - fw.num_relocs = job->num_relocs; - fw.class = 0; - bitmap_zero(waitchk_mask, host1x_syncpt_nb_pts(host)); for (i = 0; i < job->num_waitchk; i++) { u32 syncpt_id = job->waitchk[i].syncpt_id; @@ -536,20 +534,11 @@ int host1x_job_pin(struct host1x_job *job, struct device *dev) if (job->gathers[j].bo == g->bo) job->gathers[j].handled = true; - err = 0; - - if (IS_ENABLED(CONFIG_TEGRA_HOST1X_FIREWALL)) - err = validate(&fw, g); - + err = do_relocs(job, g->bo); if (err) - dev_err(dev, "Job invalid (err=%d)\n", err); - - if (!err) - err = do_relocs(job, g->bo); - - if (!err) - err = do_waitchks(job, host, g->bo); + break; + err = do_waitchks(job, host, g->bo); if (err) break; } From edeabfcbc150a48e56dd411195ef812134983d6f Mon Sep 17 00:00:00 2001 From: Arto Merilainen Date: Wed, 29 May 2013 13:26:06 +0300 Subject: [PATCH 14/16] gpu: host1x: Fix memory access in syncpt request This patch fixes a bad memory access in syncpoint request code. If no syncpoints were available, the code accessed unreserved memory area causing unexpected behaviour. Signed-off-by: Arto Merilainen Acked-By: Terje Bergstrom Signed-off-by: Thierry Reding --- drivers/gpu/host1x/syncpt.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/host1x/syncpt.c b/drivers/gpu/host1x/syncpt.c index 4b493453e805..2b03f1b5cc5a 100644 --- a/drivers/gpu/host1x/syncpt.c +++ b/drivers/gpu/host1x/syncpt.c @@ -40,7 +40,8 @@ static struct host1x_syncpt *_host1x_syncpt_alloc(struct host1x *host, for (i = 0; i < host->info->nb_pts && sp->name; i++, sp++) ; - if (sp->dev) + + if (i >= host->info->nb_pts) return NULL; name = kasprintf(GFP_KERNEL, "%02d-%s", sp->id, From ece66891ff452d5643ac5a61649f632984d83c10 Mon Sep 17 00:00:00 2001 From: Arto Merilainen Date: Wed, 29 May 2013 13:26:07 +0300 Subject: [PATCH 15/16] gpu: host1x: Fix client_managed type client_managed field in syncpoint structure was defined as an integer. The field holds, however, only a boolean value. This patch modifies the type to boolean. Signed-off-by: Arto Merilainen Acked-By: Terje Bergstrom Signed-off-by: Thierry Reding --- drivers/gpu/host1x/drm/gr2d.c | 2 +- drivers/gpu/host1x/syncpt.c | 8 ++++---- drivers/gpu/host1x/syncpt.h | 6 +++--- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/host1x/drm/gr2d.c b/drivers/gpu/host1x/drm/gr2d.c index 7efd97b2aaa4..27ffcf15a4b4 100644 --- a/drivers/gpu/host1x/drm/gr2d.c +++ b/drivers/gpu/host1x/drm/gr2d.c @@ -285,7 +285,7 @@ static int gr2d_probe(struct platform_device *pdev) if (!gr2d->channel) return -ENOMEM; - *syncpts = host1x_syncpt_request(dev, 0); + *syncpts = host1x_syncpt_request(dev, false); if (!(*syncpts)) { host1x_channel_free(gr2d->channel); return -ENOMEM; diff --git a/drivers/gpu/host1x/syncpt.c b/drivers/gpu/host1x/syncpt.c index 2b03f1b5cc5a..27201b51d808 100644 --- a/drivers/gpu/host1x/syncpt.c +++ b/drivers/gpu/host1x/syncpt.c @@ -32,7 +32,7 @@ static struct host1x_syncpt *_host1x_syncpt_alloc(struct host1x *host, struct device *dev, - int client_managed) + bool client_managed) { int i; struct host1x_syncpt *sp = host->syncpt; @@ -332,7 +332,7 @@ int host1x_syncpt_init(struct host1x *host) host1x_syncpt_restore(host); /* Allocate sync point to use for clearing waits for expired fences */ - host->nop_sp = _host1x_syncpt_alloc(host, NULL, 0); + host->nop_sp = _host1x_syncpt_alloc(host, NULL, false); if (!host->nop_sp) return -ENOMEM; @@ -340,7 +340,7 @@ int host1x_syncpt_init(struct host1x *host) } struct host1x_syncpt *host1x_syncpt_request(struct device *dev, - int client_managed) + bool client_managed) { struct host1x *host = dev_get_drvdata(dev->parent); return _host1x_syncpt_alloc(host, dev, client_managed); @@ -354,7 +354,7 @@ void host1x_syncpt_free(struct host1x_syncpt *sp) kfree(sp->name); sp->dev = NULL; sp->name = NULL; - sp->client_managed = 0; + sp->client_managed = false; } void host1x_syncpt_deinit(struct host1x *host) diff --git a/drivers/gpu/host1x/syncpt.h b/drivers/gpu/host1x/syncpt.h index c99806130f2e..d00e758352eb 100644 --- a/drivers/gpu/host1x/syncpt.h +++ b/drivers/gpu/host1x/syncpt.h @@ -36,7 +36,7 @@ struct host1x_syncpt { atomic_t max_val; u32 base_val; const char *name; - int client_managed; + bool client_managed; struct host1x *host; struct device *dev; @@ -94,7 +94,7 @@ static inline bool host1x_syncpt_check_max(struct host1x_syncpt *sp, u32 real) } /* Return true if sync point is client managed. */ -static inline int host1x_syncpt_client_managed(struct host1x_syncpt *sp) +static inline bool host1x_syncpt_client_managed(struct host1x_syncpt *sp) { return sp->client_managed; } @@ -157,7 +157,7 @@ u32 host1x_syncpt_id(struct host1x_syncpt *sp); /* Allocate a sync point for a device. */ struct host1x_syncpt *host1x_syncpt_request(struct device *dev, - int client_managed); + bool client_managed); /* Free a sync point. */ void host1x_syncpt_free(struct host1x_syncpt *sp); From ebae30b1fbcc2cc991ce705cc82e16d1e5ddbf51 Mon Sep 17 00:00:00 2001 From: Arto Merilainen Date: Wed, 29 May 2013 13:26:08 +0300 Subject: [PATCH 16/16] gpu: host1x: Rework CPU syncpoint increment This patch merges host1x_syncpt_cpu_incr to host1x_syncpt_incr() as they are in practise doing the same thing. host1x_syncpt_incr() is also modified to return error codes. User space interface is modified accordingly to pass return values. Signed-off-by: Arto Merilainen Acked-By: Terje Bergstrom Signed-off-by: Thierry Reding --- drivers/gpu/host1x/dev.h | 8 ++++---- drivers/gpu/host1x/drm/drm.c | 3 +-- drivers/gpu/host1x/hw/cdma_hw.c | 2 +- drivers/gpu/host1x/hw/syncpt_hw.c | 12 +++++------- drivers/gpu/host1x/syncpt.c | 15 ++------------- drivers/gpu/host1x/syncpt.h | 7 ++----- 6 files changed, 15 insertions(+), 32 deletions(-) diff --git a/drivers/gpu/host1x/dev.h b/drivers/gpu/host1x/dev.h index a1607d6e135b..790ddf114e58 100644 --- a/drivers/gpu/host1x/dev.h +++ b/drivers/gpu/host1x/dev.h @@ -73,7 +73,7 @@ struct host1x_syncpt_ops { void (*restore_wait_base)(struct host1x_syncpt *syncpt); void (*load_wait_base)(struct host1x_syncpt *syncpt); u32 (*load)(struct host1x_syncpt *syncpt); - void (*cpu_incr)(struct host1x_syncpt *syncpt); + int (*cpu_incr)(struct host1x_syncpt *syncpt); int (*patch_wait)(struct host1x_syncpt *syncpt, void *patch_addr); }; @@ -157,10 +157,10 @@ static inline u32 host1x_hw_syncpt_load(struct host1x *host, return host->syncpt_op->load(sp); } -static inline void host1x_hw_syncpt_cpu_incr(struct host1x *host, - struct host1x_syncpt *sp) +static inline int host1x_hw_syncpt_cpu_incr(struct host1x *host, + struct host1x_syncpt *sp) { - host->syncpt_op->cpu_incr(sp); + return host->syncpt_op->cpu_incr(sp); } static inline int host1x_hw_syncpt_patch_wait(struct host1x *host, diff --git a/drivers/gpu/host1x/drm/drm.c b/drivers/gpu/host1x/drm/drm.c index c171a07f47bf..e184b00faacd 100644 --- a/drivers/gpu/host1x/drm/drm.c +++ b/drivers/gpu/host1x/drm/drm.c @@ -387,8 +387,7 @@ static int tegra_syncpt_incr(struct drm_device *drm, void *data, if (!sp) return -EINVAL; - host1x_syncpt_incr(sp); - return 0; + return host1x_syncpt_incr(sp); } static int tegra_syncpt_wait(struct drm_device *drm, void *data, diff --git a/drivers/gpu/host1x/hw/cdma_hw.c b/drivers/gpu/host1x/hw/cdma_hw.c index 590b69d91dab..2ee4ad55c4db 100644 --- a/drivers/gpu/host1x/hw/cdma_hw.c +++ b/drivers/gpu/host1x/hw/cdma_hw.c @@ -44,7 +44,7 @@ static void cdma_timeout_cpu_incr(struct host1x_cdma *cdma, u32 getptr, u32 i; for (i = 0; i < syncpt_incrs; i++) - host1x_syncpt_cpu_incr(cdma->timeout.syncpt); + host1x_syncpt_incr(cdma->timeout.syncpt); /* after CPU incr, ensure shadow is up to date */ host1x_syncpt_load(cdma->timeout.syncpt); diff --git a/drivers/gpu/host1x/hw/syncpt_hw.c b/drivers/gpu/host1x/hw/syncpt_hw.c index 61174990102a..0cf6095d3367 100644 --- a/drivers/gpu/host1x/hw/syncpt_hw.c +++ b/drivers/gpu/host1x/hw/syncpt_hw.c @@ -77,21 +77,19 @@ static u32 syncpt_load(struct host1x_syncpt *sp) * Write a cpu syncpoint increment to the hardware, without touching * the cache. */ -static void syncpt_cpu_incr(struct host1x_syncpt *sp) +static int syncpt_cpu_incr(struct host1x_syncpt *sp) { struct host1x *host = sp->host; u32 reg_offset = sp->id / 32; if (!host1x_syncpt_client_managed(sp) && - host1x_syncpt_idle(sp)) { - dev_err(host->dev, "Trying to increment syncpoint id %d beyond max\n", - sp->id); - host1x_debug_dump(sp->host); - return; - } + host1x_syncpt_idle(sp)) + return -EINVAL; host1x_sync_writel(host, BIT_MASK(sp->id), HOST1X_SYNC_SYNCPT_CPU_INCR(reg_offset)); wmb(); + + return 0; } /* remove a wait pointed to by patch_addr */ diff --git a/drivers/gpu/host1x/syncpt.c b/drivers/gpu/host1x/syncpt.c index 27201b51d808..409745b949db 100644 --- a/drivers/gpu/host1x/syncpt.c +++ b/drivers/gpu/host1x/syncpt.c @@ -128,23 +128,12 @@ u32 host1x_syncpt_load_wait_base(struct host1x_syncpt *sp) return val; } -/* - * Write a cpu syncpoint increment to the hardware, without touching - * the cache. Caller is responsible for host being powered. - */ -void host1x_syncpt_cpu_incr(struct host1x_syncpt *sp) -{ - host1x_hw_syncpt_cpu_incr(sp->host, sp); -} - /* * Increment syncpoint value from cpu, updating cache */ -void host1x_syncpt_incr(struct host1x_syncpt *sp) +int host1x_syncpt_incr(struct host1x_syncpt *sp) { - if (host1x_syncpt_client_managed(sp)) - host1x_syncpt_incr_max(sp, 1); - host1x_syncpt_cpu_incr(sp); + return host1x_hw_syncpt_cpu_incr(sp->host, sp); } /* diff --git a/drivers/gpu/host1x/syncpt.h b/drivers/gpu/host1x/syncpt.h index d00e758352eb..267c0b9d3647 100644 --- a/drivers/gpu/host1x/syncpt.h +++ b/drivers/gpu/host1x/syncpt.h @@ -115,9 +115,6 @@ static inline bool host1x_syncpt_idle(struct host1x_syncpt *sp) /* Return pointer to struct denoting sync point id. */ struct host1x_syncpt *host1x_syncpt_get(struct host1x *host, u32 id); -/* Request incrementing a sync point. */ -void host1x_syncpt_cpu_incr(struct host1x_syncpt *sp); - /* Load current value from hardware to the shadow register. */ u32 host1x_syncpt_load(struct host1x_syncpt *sp); @@ -133,8 +130,8 @@ void host1x_syncpt_restore(struct host1x *host); /* Read current wait base value into shadow register and return it. */ u32 host1x_syncpt_load_wait_base(struct host1x_syncpt *sp); -/* Increment sync point and its max. */ -void host1x_syncpt_incr(struct host1x_syncpt *sp); +/* Request incrementing a sync point. */ +int host1x_syncpt_incr(struct host1x_syncpt *sp); /* Indicate future operations by incrementing the sync point max. */ u32 host1x_syncpt_incr_max(struct host1x_syncpt *sp, u32 incrs);