From d8f2ebaac650dc35db3bf5cf10e8ee1115b455f8 Mon Sep 17 00:00:00 2001 From: Dave Airlie Date: Thu, 13 Apr 2017 11:41:38 +1000 Subject: sync_file: get rid of internal reference count. sync_file uses the reference count of the file, the internal kref was never getting moved past 1. We can reintroduce this if we decide we need it later. [airlied: fix buildbot warnings] Reviewed-by: Chris Wilson Signed-off-by: Dave Airlie Acked-by: Sumit Semwal Signed-off-by: Gustavo Padovan Link: http://patchwork.freedesktop.org/patch/msgid/20170413014144.637-2-airlied@gmail.com --- include/linux/sync_file.h | 3 --- 1 file changed, 3 deletions(-) (limited to 'include/linux') diff --git a/include/linux/sync_file.h b/include/linux/sync_file.h index 3e3ab84fc4cd..d37beefdfbd5 100644 --- a/include/linux/sync_file.h +++ b/include/linux/sync_file.h @@ -14,7 +14,6 @@ #define _LINUX_SYNC_FILE_H #include -#include #include #include #include @@ -24,7 +23,6 @@ /** * struct sync_file - sync file to export to the userspace * @file: file representing this fence - * @kref: reference count on fence. * @name: name of sync_file. Useful for debugging * @sync_file_list: membership in global file list * @wq: wait queue for fence signaling @@ -33,7 +31,6 @@ */ struct sync_file { struct file *file; - struct kref kref; char name[32]; #ifdef CONFIG_DEBUG_FS struct list_head sync_file_list; -- cgit v1.2.3 From 73c73463189974ace90a05397197339071c6ecc7 Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Wed, 12 Apr 2017 20:17:45 -0700 Subject: video: ARM CLCD: Move registers to a separate header. We'd like to reuse these register definitions for the DRM CLCD driver, but there's a bunch of fbdev-specific code in the current header. v2: Add #ifndef guard. Signed-off-by: Eric Anholt Link: http://patchwork.freedesktop.org/patch/msgid/20170413031746.12921-1-eric@anholt.net --- include/linux/amba/clcd-regs.h | 81 ++++++++++++++++++++++++++++++++++++++++++ include/linux/amba/clcd.h | 68 +---------------------------------- 2 files changed, 82 insertions(+), 67 deletions(-) create mode 100644 include/linux/amba/clcd-regs.h (limited to 'include/linux') diff --git a/include/linux/amba/clcd-regs.h b/include/linux/amba/clcd-regs.h new file mode 100644 index 000000000000..69c0e2143003 --- /dev/null +++ b/include/linux/amba/clcd-regs.h @@ -0,0 +1,81 @@ +/* + * David A Rusling + * + * Copyright (C) 2001 ARM Limited + * + * This file is subject to the terms and conditions of the GNU General Public + * License. See the file COPYING in the main directory of this archive + * for more details. + */ + +#ifndef AMBA_CLCD_REGS_H +#define AMBA_CLCD_REGS_H + +/* + * CLCD Controller Internal Register addresses + */ +#define CLCD_TIM0 0x00000000 +#define CLCD_TIM1 0x00000004 +#define CLCD_TIM2 0x00000008 +#define CLCD_TIM3 0x0000000c +#define CLCD_UBAS 0x00000010 +#define CLCD_LBAS 0x00000014 + +#define CLCD_PL110_IENB 0x00000018 +#define CLCD_PL110_CNTL 0x0000001c +#define CLCD_PL110_STAT 0x00000020 +#define CLCD_PL110_INTR 0x00000024 +#define CLCD_PL110_UCUR 0x00000028 +#define CLCD_PL110_LCUR 0x0000002C + +#define CLCD_PL111_CNTL 0x00000018 +#define CLCD_PL111_IENB 0x0000001c +#define CLCD_PL111_RIS 0x00000020 +#define CLCD_PL111_MIS 0x00000024 +#define CLCD_PL111_ICR 0x00000028 +#define CLCD_PL111_UCUR 0x0000002c +#define CLCD_PL111_LCUR 0x00000030 + +#define CLCD_PALL 0x00000200 +#define CLCD_PALETTE 0x00000200 + +#define TIM2_CLKSEL (1 << 5) +#define TIM2_IVS (1 << 11) +#define TIM2_IHS (1 << 12) +#define TIM2_IPC (1 << 13) +#define TIM2_IOE (1 << 14) +#define TIM2_BCD (1 << 26) + +#define CNTL_LCDEN (1 << 0) +#define CNTL_LCDBPP1 (0 << 1) +#define CNTL_LCDBPP2 (1 << 1) +#define CNTL_LCDBPP4 (2 << 1) +#define CNTL_LCDBPP8 (3 << 1) +#define CNTL_LCDBPP16 (4 << 1) +#define CNTL_LCDBPP16_565 (6 << 1) +#define CNTL_LCDBPP16_444 (7 << 1) +#define CNTL_LCDBPP24 (5 << 1) +#define CNTL_LCDBW (1 << 4) +#define CNTL_LCDTFT (1 << 5) +#define CNTL_LCDMONO8 (1 << 6) +#define CNTL_LCDDUAL (1 << 7) +#define CNTL_BGR (1 << 8) +#define CNTL_BEBO (1 << 9) +#define CNTL_BEPO (1 << 10) +#define CNTL_LCDPWR (1 << 11) +#define CNTL_LCDVCOMP(x) ((x) << 12) +#define CNTL_LDMAFIFOTIME (1 << 15) +#define CNTL_WATERMARK (1 << 16) + +/* ST Microelectronics variant bits */ +#define CNTL_ST_1XBPP_444 0x0 +#define CNTL_ST_1XBPP_5551 (1 << 17) +#define CNTL_ST_1XBPP_565 (1 << 18) +#define CNTL_ST_CDWID_12 0x0 +#define CNTL_ST_CDWID_16 (1 << 19) +#define CNTL_ST_CDWID_18 (1 << 20) +#define CNTL_ST_CDWID_24 ((1 << 19)|(1 << 20)) +#define CNTL_ST_CEAEN (1 << 21) +#define CNTL_ST_LCDBPP24_PACKED (6 << 1) + +#endif /* AMBA_CLCD_REGS_H */ diff --git a/include/linux/amba/clcd.h b/include/linux/amba/clcd.h index 1035879b322c..d0c3be77c18e 100644 --- a/include/linux/amba/clcd.h +++ b/include/linux/amba/clcd.h @@ -10,73 +10,7 @@ * for more details. */ #include - -/* - * CLCD Controller Internal Register addresses - */ -#define CLCD_TIM0 0x00000000 -#define CLCD_TIM1 0x00000004 -#define CLCD_TIM2 0x00000008 -#define CLCD_TIM3 0x0000000c -#define CLCD_UBAS 0x00000010 -#define CLCD_LBAS 0x00000014 - -#define CLCD_PL110_IENB 0x00000018 -#define CLCD_PL110_CNTL 0x0000001c -#define CLCD_PL110_STAT 0x00000020 -#define CLCD_PL110_INTR 0x00000024 -#define CLCD_PL110_UCUR 0x00000028 -#define CLCD_PL110_LCUR 0x0000002C - -#define CLCD_PL111_CNTL 0x00000018 -#define CLCD_PL111_IENB 0x0000001c -#define CLCD_PL111_RIS 0x00000020 -#define CLCD_PL111_MIS 0x00000024 -#define CLCD_PL111_ICR 0x00000028 -#define CLCD_PL111_UCUR 0x0000002c -#define CLCD_PL111_LCUR 0x00000030 - -#define CLCD_PALL 0x00000200 -#define CLCD_PALETTE 0x00000200 - -#define TIM2_CLKSEL (1 << 5) -#define TIM2_IVS (1 << 11) -#define TIM2_IHS (1 << 12) -#define TIM2_IPC (1 << 13) -#define TIM2_IOE (1 << 14) -#define TIM2_BCD (1 << 26) - -#define CNTL_LCDEN (1 << 0) -#define CNTL_LCDBPP1 (0 << 1) -#define CNTL_LCDBPP2 (1 << 1) -#define CNTL_LCDBPP4 (2 << 1) -#define CNTL_LCDBPP8 (3 << 1) -#define CNTL_LCDBPP16 (4 << 1) -#define CNTL_LCDBPP16_565 (6 << 1) -#define CNTL_LCDBPP16_444 (7 << 1) -#define CNTL_LCDBPP24 (5 << 1) -#define CNTL_LCDBW (1 << 4) -#define CNTL_LCDTFT (1 << 5) -#define CNTL_LCDMONO8 (1 << 6) -#define CNTL_LCDDUAL (1 << 7) -#define CNTL_BGR (1 << 8) -#define CNTL_BEBO (1 << 9) -#define CNTL_BEPO (1 << 10) -#define CNTL_LCDPWR (1 << 11) -#define CNTL_LCDVCOMP(x) ((x) << 12) -#define CNTL_LDMAFIFOTIME (1 << 15) -#define CNTL_WATERMARK (1 << 16) - -/* ST Microelectronics variant bits */ -#define CNTL_ST_1XBPP_444 0x0 -#define CNTL_ST_1XBPP_5551 (1 << 17) -#define CNTL_ST_1XBPP_565 (1 << 18) -#define CNTL_ST_CDWID_12 0x0 -#define CNTL_ST_CDWID_16 (1 << 19) -#define CNTL_ST_CDWID_18 (1 << 20) -#define CNTL_ST_CDWID_24 ((1 << 19)|(1 << 20)) -#define CNTL_ST_CEAEN (1 << 21) -#define CNTL_ST_LCDBPP24_PACKED (6 << 1) +#include enum { /* individual formats */ -- cgit v1.2.3 From 032838f9cb4014af8a974374db9e2ce6f3aa8d3b Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Mon, 8 May 2017 12:33:48 -0700 Subject: drm/pl111: Register the clock divider and use it. This is required for the panel to work on bcm911360, where CLCDCLK is the fixed 200Mhz AXI41 clock. The rate set is still passed up to the CLCDCLK, for platforms that have a settable rate on that one. v2: Set SET_RATE_PARENT (caught by Linus Walleij), depend on COMMON_CLK. v3: Mark the clk_ops static (caught by Stephen). Signed-off-by: Eric Anholt Link: http://patchwork.freedesktop.org/patch/msgid/20170508193348.30236-1-eric@anholt.net Reviewed-by: Linus Walleij Reviewed-by: Stephen Boyd --- drivers/gpu/drm/pl111/Kconfig | 1 + drivers/gpu/drm/pl111/pl111_display.c | 162 ++++++++++++++++++++++++++++++---- drivers/gpu/drm/pl111/pl111_drm.h | 8 ++ drivers/gpu/drm/pl111/pl111_drv.c | 11 +-- include/linux/amba/clcd-regs.h | 5 ++ 5 files changed, 163 insertions(+), 24 deletions(-) (limited to 'include/linux') diff --git a/drivers/gpu/drm/pl111/Kconfig b/drivers/gpu/drm/pl111/Kconfig index ede49efd531f..309f4fd52de7 100644 --- a/drivers/gpu/drm/pl111/Kconfig +++ b/drivers/gpu/drm/pl111/Kconfig @@ -2,6 +2,7 @@ config DRM_PL111 tristate "DRM Support for PL111 CLCD Controller" depends on DRM depends on ARM || ARM64 || COMPILE_TEST + depends on COMMON_CLK select DRM_KMS_HELPER select DRM_KMS_CMA_HELPER select DRM_GEM_CMA_HELPER diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c index 39a5c33bce7d..fbf8fbec6c16 100644 --- a/drivers/gpu/drm/pl111/pl111_display.c +++ b/drivers/gpu/drm/pl111/pl111_display.c @@ -108,7 +108,7 @@ static void pl111_display_enable(struct drm_simple_display_pipe *pipe, u32 cntl; u32 ppl, hsw, hfp, hbp; u32 lpp, vsw, vfp, vbp; - u32 cpl; + u32 cpl, tim2; int ret; ret = clk_set_rate(priv->clk, mode->clock * 1000); @@ -142,20 +142,28 @@ static void pl111_display_enable(struct drm_simple_display_pipe *pipe, (vfp << 16) | (vbp << 24), priv->regs + CLCD_TIM1); - /* XXX: We currently always use CLCDCLK with no divisor. We - * could probably reduce power consumption by using HCLK - * (apb_pclk) with a divisor when it gets us near our target - * pixel clock. - */ - writel(((mode->flags & DRM_MODE_FLAG_NHSYNC) ? TIM2_IHS : 0) | - ((mode->flags & DRM_MODE_FLAG_NVSYNC) ? TIM2_IVS : 0) | - ((connector->display_info.bus_flags & - DRM_BUS_FLAG_DE_LOW) ? TIM2_IOE : 0) | - ((connector->display_info.bus_flags & - DRM_BUS_FLAG_PIXDATA_NEGEDGE) ? TIM2_IPC : 0) | - TIM2_BCD | - (cpl << 16), - priv->regs + CLCD_TIM2); + + spin_lock(&priv->tim2_lock); + + tim2 = readl(priv->regs + CLCD_TIM2); + tim2 &= (TIM2_BCD | TIM2_PCD_LO_MASK | TIM2_PCD_HI_MASK); + + if (mode->flags & DRM_MODE_FLAG_NHSYNC) + tim2 |= TIM2_IHS; + + if (mode->flags & DRM_MODE_FLAG_NVSYNC) + tim2 |= TIM2_IVS; + + if (connector->display_info.bus_flags & DRM_BUS_FLAG_DE_LOW) + tim2 |= TIM2_IOE; + + if (connector->display_info.bus_flags & DRM_BUS_FLAG_PIXDATA_NEGEDGE) + tim2 |= TIM2_IPC; + + tim2 |= cpl << 16; + writel(tim2, priv->regs + CLCD_TIM2); + spin_unlock(&priv->tim2_lock); + writel(0, priv->regs + CLCD_TIM3); drm_panel_prepare(priv->connector.panel); @@ -288,6 +296,126 @@ const struct drm_simple_display_pipe_funcs pl111_display_funcs = { .prepare_fb = pl111_display_prepare_fb, }; +static int pl111_clk_div_choose_div(struct clk_hw *hw, unsigned long rate, + unsigned long *prate, bool set_parent) +{ + int best_div = 1, div; + struct clk_hw *parent = clk_hw_get_parent(hw); + unsigned long best_prate = 0; + unsigned long best_diff = ~0ul; + int max_div = (1 << (TIM2_PCD_LO_BITS + TIM2_PCD_HI_BITS)) - 1; + + for (div = 1; div < max_div; div++) { + unsigned long this_prate, div_rate, diff; + + if (set_parent) + this_prate = clk_hw_round_rate(parent, rate * div); + else + this_prate = *prate; + div_rate = DIV_ROUND_UP_ULL(this_prate, div); + diff = abs(rate - div_rate); + + if (diff < best_diff) { + best_div = div; + best_diff = diff; + best_prate = this_prate; + } + } + + *prate = best_prate; + return best_div; +} + +static long pl111_clk_div_round_rate(struct clk_hw *hw, unsigned long rate, + unsigned long *prate) +{ + int div = pl111_clk_div_choose_div(hw, rate, prate, true); + + return DIV_ROUND_UP_ULL(*prate, div); +} + +static unsigned long pl111_clk_div_recalc_rate(struct clk_hw *hw, + unsigned long prate) +{ + struct pl111_drm_dev_private *priv = + container_of(hw, struct pl111_drm_dev_private, clk_div); + u32 tim2 = readl(priv->regs + CLCD_TIM2); + int div; + + if (tim2 & TIM2_BCD) + return prate; + + div = tim2 & TIM2_PCD_LO_MASK; + div |= (tim2 & TIM2_PCD_HI_MASK) >> + (TIM2_PCD_HI_SHIFT - TIM2_PCD_LO_BITS); + div += 2; + + return DIV_ROUND_UP_ULL(prate, div); +} + +static int pl111_clk_div_set_rate(struct clk_hw *hw, unsigned long rate, + unsigned long prate) +{ + struct pl111_drm_dev_private *priv = + container_of(hw, struct pl111_drm_dev_private, clk_div); + int div = pl111_clk_div_choose_div(hw, rate, &prate, false); + u32 tim2; + + spin_lock(&priv->tim2_lock); + tim2 = readl(priv->regs + CLCD_TIM2); + tim2 &= ~(TIM2_BCD | TIM2_PCD_LO_MASK | TIM2_PCD_HI_MASK); + + if (div == 1) { + tim2 |= TIM2_BCD; + } else { + div -= 2; + tim2 |= div & TIM2_PCD_LO_MASK; + tim2 |= (div >> TIM2_PCD_LO_BITS) << TIM2_PCD_HI_SHIFT; + } + + writel(tim2, priv->regs + CLCD_TIM2); + spin_unlock(&priv->tim2_lock); + + return 0; +} + +static const struct clk_ops pl111_clk_div_ops = { + .recalc_rate = pl111_clk_div_recalc_rate, + .round_rate = pl111_clk_div_round_rate, + .set_rate = pl111_clk_div_set_rate, +}; + +static int +pl111_init_clock_divider(struct drm_device *drm) +{ + struct pl111_drm_dev_private *priv = drm->dev_private; + struct clk *parent = devm_clk_get(drm->dev, "clcdclk"); + struct clk_hw *div = &priv->clk_div; + const char *parent_name; + struct clk_init_data init = { + .name = "pl111_div", + .ops = &pl111_clk_div_ops, + .parent_names = &parent_name, + .num_parents = 1, + .flags = CLK_SET_RATE_PARENT, + }; + int ret; + + if (IS_ERR(parent)) { + dev_err(drm->dev, "CLCD: unable to get clcdclk.\n"); + return PTR_ERR(parent); + } + parent_name = __clk_get_name(parent); + + spin_lock_init(&priv->tim2_lock); + div->init = &init; + + ret = devm_clk_hw_register(drm->dev, div); + + priv->clk = div->clk; + return ret; +} + int pl111_display_init(struct drm_device *drm) { struct pl111_drm_dev_private *priv = drm->dev_private; @@ -333,6 +461,10 @@ int pl111_display_init(struct drm_device *drm) return -EINVAL; } + ret = pl111_init_clock_divider(drm); + if (ret) + return ret; + ret = drm_simple_display_pipe_init(drm, &priv->pipe, &pl111_display_funcs, formats, ARRAY_SIZE(formats), diff --git a/drivers/gpu/drm/pl111/pl111_drm.h b/drivers/gpu/drm/pl111/pl111_drm.h index f381593921b7..4162c6aa5dbb 100644 --- a/drivers/gpu/drm/pl111/pl111_drm.h +++ b/drivers/gpu/drm/pl111/pl111_drm.h @@ -21,6 +21,7 @@ #include #include +#include #define CLCD_IRQ_NEXTBASE_UPDATE BIT(2) @@ -37,7 +38,14 @@ struct pl111_drm_dev_private { struct drm_fbdev_cma *fbdev; void *regs; + /* The pixel clock (a reference to our clock divider off of CLCDCLK). */ struct clk *clk; + /* pl111's internal clock divider. */ + struct clk_hw clk_div; + /* Lock to sync access to CLCD_TIM2 between the common clock + * subsystem and pl111_display_enable(). + */ + spinlock_t tim2_lock; }; #define to_pl111_connector(x) \ diff --git a/drivers/gpu/drm/pl111/pl111_drv.c b/drivers/gpu/drm/pl111/pl111_drv.c index 936403f65508..9d1467492cb9 100644 --- a/drivers/gpu/drm/pl111/pl111_drv.c +++ b/drivers/gpu/drm/pl111/pl111_drv.c @@ -50,8 +50,8 @@ * - Read back hardware state at boot to skip reprogramming the * hardware when doing a no-op modeset. * - * - Use the internal clock divisor to reduce power consumption by - * using HCLK (apb_pclk) when appropriate. + * - Use the CLKSEL bit to support switching between the two external + * clock parents. */ #include @@ -195,13 +195,6 @@ static int pl111_amba_probe(struct amba_device *amba_dev, priv->drm = drm; drm->dev_private = priv; - priv->clk = devm_clk_get(dev, "clcdclk"); - if (IS_ERR(priv->clk)) { - dev_err(dev, "CLCD: unable to get clk.\n"); - ret = PTR_ERR(priv->clk); - goto dev_unref; - } - priv->regs = devm_ioremap_resource(dev, &amba_dev->res); if (!priv->regs) { dev_err(dev, "%s failed mmio\n", __func__); diff --git a/include/linux/amba/clcd-regs.h b/include/linux/amba/clcd-regs.h index 69c0e2143003..516a6fda83c5 100644 --- a/include/linux/amba/clcd-regs.h +++ b/include/linux/amba/clcd-regs.h @@ -39,12 +39,17 @@ #define CLCD_PALL 0x00000200 #define CLCD_PALETTE 0x00000200 +#define TIM2_PCD_LO_MASK GENMASK(4, 0) +#define TIM2_PCD_LO_BITS 5 #define TIM2_CLKSEL (1 << 5) #define TIM2_IVS (1 << 11) #define TIM2_IHS (1 << 12) #define TIM2_IPC (1 << 13) #define TIM2_IOE (1 << 14) #define TIM2_BCD (1 << 26) +#define TIM2_PCD_HI_MASK GENMASK(31, 27) +#define TIM2_PCD_HI_BITS 5 +#define TIM2_PCD_HI_SHIFT 27 #define CNTL_LCDEN (1 << 0) #define CNTL_LCDBPP1 (0 << 1) -- cgit v1.2.3 From 71ebc9a3795818eab52e81bbcbdfae130ee35d9e Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Tue, 16 May 2017 12:10:42 +0100 Subject: dma-buf/sync-file: Defer creation of sync_file->name Constructing the name takes the majority of the time for allocating a sync_file to wrap a fence, and the name is very rarely used (only via the sync_file status user interface). To reduce the impact on the common path (that of creating sync_file to pass around), defer the construction of the name until it is first used. v2: Update kerneldoc (kbuild test robot) v3: sync_debug.c was peeking at the name v4: Comment upon the potential race between two users of sync_file_get_name() and claim that such a race is below the level of notice. However, to prevent any future nuisance, use a global spinlock to serialize the assignment of the name. v5: Completely avoid the read/write race by only storing the name passed in from the user inside sync_file->user_name and passing in a buffer to dynamically construct the name otherwise. Signed-off-by: Chris Wilson Cc: Sumit Semwal Cc: Gustavo Padovan Cc: Daniel Vetter Cc: David Herrmann Reviewed-by: Daniel Vetter Signed-off-by: Gustavo Padovan Link: http://patchwork.freedesktop.org/patch/msgid/20170516111042.24719-1-chris@chris-wilson.co.uk --- drivers/dma-buf/sync_debug.c | 4 +++- drivers/dma-buf/sync_file.c | 39 ++++++++++++++++++++++++++++++++------- include/linux/sync_file.h | 11 +++++++++-- 3 files changed, 44 insertions(+), 10 deletions(-) (limited to 'include/linux') diff --git a/drivers/dma-buf/sync_debug.c b/drivers/dma-buf/sync_debug.c index a0d780ab68c3..82a6e7f6d37f 100644 --- a/drivers/dma-buf/sync_debug.c +++ b/drivers/dma-buf/sync_debug.c @@ -132,9 +132,11 @@ static void sync_print_obj(struct seq_file *s, struct sync_timeline *obj) static void sync_print_sync_file(struct seq_file *s, struct sync_file *sync_file) { + char buf[128]; int i; - seq_printf(s, "[%p] %s: %s\n", sync_file, sync_file->name, + seq_printf(s, "[%p] %s: %s\n", sync_file, + sync_file_get_name(sync_file, buf, sizeof(buf)), sync_status_str(dma_fence_get_status(sync_file->fence))); if (dma_fence_is_array(sync_file->fence)) { diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c index dc89b1d484e8..545e2c5c4815 100644 --- a/drivers/dma-buf/sync_file.c +++ b/drivers/dma-buf/sync_file.c @@ -80,11 +80,6 @@ struct sync_file *sync_file_create(struct dma_fence *fence) sync_file->fence = dma_fence_get(fence); - snprintf(sync_file->name, sizeof(sync_file->name), "%s-%s%llu-%d", - fence->ops->get_driver_name(fence), - fence->ops->get_timeline_name(fence), fence->context, - fence->seqno); - return sync_file; } EXPORT_SYMBOL(sync_file_create); @@ -129,6 +124,36 @@ struct dma_fence *sync_file_get_fence(int fd) } EXPORT_SYMBOL(sync_file_get_fence); +/** + * sync_file_get_name - get the name of the sync_file + * @sync_file: sync_file to get the fence from + * @buf: destination buffer to copy sync_file name into + * @len: available size of destination buffer. + * + * Each sync_file may have a name assigned either by the user (when merging + * sync_files together) or created from the fence it contains. In the latter + * case construction of the name is deferred until use, and so requires + * sync_file_get_name(). + * + * Returns: a string representing the name. + */ +char *sync_file_get_name(struct sync_file *sync_file, char *buf, int len) +{ + if (sync_file->user_name[0]) { + strlcpy(buf, sync_file->user_name, len); + } else { + struct dma_fence *fence = sync_file->fence; + + snprintf(buf, len, "%s-%s%llu-%d", + fence->ops->get_driver_name(fence), + fence->ops->get_timeline_name(fence), + fence->context, + fence->seqno); + } + + return buf; +} + static int sync_file_set_fence(struct sync_file *sync_file, struct dma_fence **fences, int num_fences) { @@ -266,7 +291,7 @@ static struct sync_file *sync_file_merge(const char *name, struct sync_file *a, goto err; } - strlcpy(sync_file->name, name, sizeof(sync_file->name)); + strlcpy(sync_file->user_name, name, sizeof(sync_file->user_name)); return sync_file; err: @@ -413,7 +438,7 @@ static long sync_file_ioctl_fence_info(struct sync_file *sync_file, } no_fences: - strlcpy(info.name, sync_file->name, sizeof(info.name)); + sync_file_get_name(sync_file, info.name, sizeof(info.name)); info.status = dma_fence_is_signaled(sync_file->fence); info.num_fences = num_fences; diff --git a/include/linux/sync_file.h b/include/linux/sync_file.h index d37beefdfbd5..5726107963b2 100644 --- a/include/linux/sync_file.h +++ b/include/linux/sync_file.h @@ -23,7 +23,6 @@ /** * struct sync_file - sync file to export to the userspace * @file: file representing this fence - * @name: name of sync_file. Useful for debugging * @sync_file_list: membership in global file list * @wq: wait queue for fence signaling * @fence: fence with the fences in the sync_file @@ -31,7 +30,14 @@ */ struct sync_file { struct file *file; - char name[32]; + /** + * @user_name: + * + * Name of the sync file provided by userspace, for merged fences. + * Otherwise generated through driver callbacks (in which case the + * entire array is 0). + */ + char user_name[32]; #ifdef CONFIG_DEBUG_FS struct list_head sync_file_list; #endif @@ -46,5 +52,6 @@ struct sync_file { struct sync_file *sync_file_create(struct dma_fence *fence); struct dma_fence *sync_file_get_fence(int fd); +char *sync_file_get_name(struct sync_file *sync_file, char *buf, int len); #endif /* _LINUX_SYNC_H */ -- cgit v1.2.3 From 278cba7eaf5422510fc4a6b5a4d447f17b00506e Mon Sep 17 00:00:00 2001 From: Laurent Pinchart Date: Wed, 14 Dec 2016 03:35:03 +0200 Subject: drm: omapdrm: Remove unused default display name support The default display name is both unused and never set by platform data. Remove default display name module parameter, platform data field and runtime infrastructure. Signed-off-by: Laurent Pinchart Acked-by: Bartlomiej Zolnierkiewicz Reviewed-by: Tomi Valkeinen Signed-off-by: Tomi Valkeinen --- drivers/gpu/drm/omapdrm/dss/core.c | 18 ------------------ drivers/gpu/drm/omapdrm/dss/omapdss.h | 1 - drivers/video/fbdev/omap2/omapfb/dss/core.c | 2 -- include/linux/platform_data/omapdss.h | 1 - 4 files changed, 22 deletions(-) (limited to 'include/linux') diff --git a/drivers/gpu/drm/omapdrm/dss/core.c b/drivers/gpu/drm/omapdrm/dss/core.c index 3351ce23f413..bc4fa4ec8557 100644 --- a/drivers/gpu/drm/omapdrm/dss/core.c +++ b/drivers/gpu/drm/omapdrm/dss/core.c @@ -41,20 +41,8 @@ static struct { struct platform_device *pdev; - - const char *default_display_name; } core; -static char *def_disp_name; -module_param_named(def_disp, def_disp_name, charp, 0); -MODULE_PARM_DESC(def_disp, "default display name"); - -const char *omapdss_get_default_display_name(void) -{ - return core.default_display_name; -} -EXPORT_SYMBOL(omapdss_get_default_display_name); - enum omapdss_version omapdss_get_version(void) { struct omap_dss_board_info *pdata = core.pdev->dev.platform_data; @@ -175,7 +163,6 @@ static void dss_disable_all_devices(void) static int __init omap_dss_probe(struct platform_device *pdev) { - struct omap_dss_board_info *pdata = pdev->dev.platform_data; int r; core.pdev = pdev; @@ -186,11 +173,6 @@ static int __init omap_dss_probe(struct platform_device *pdev) if (r) goto err_debugfs; - if (def_disp_name) - core.default_display_name = def_disp_name; - else if (pdata->default_display_name) - core.default_display_name = pdata->default_display_name; - return 0; err_debugfs: diff --git a/drivers/gpu/drm/omapdrm/dss/omapdss.h b/drivers/gpu/drm/omapdrm/dss/omapdss.h index b19dae1fd6c5..3bee528380cd 100644 --- a/drivers/gpu/drm/omapdrm/dss/omapdss.h +++ b/drivers/gpu/drm/omapdrm/dss/omapdss.h @@ -781,7 +781,6 @@ void omap_dss_put_device(struct omap_dss_device *dssdev); struct omap_dss_device *omap_dss_get_next_device(struct omap_dss_device *from); struct omap_dss_device *omap_dss_find_device(void *data, int (*match)(struct omap_dss_device *dssdev, void *data)); -const char *omapdss_get_default_display_name(void); int dss_feat_get_num_mgrs(void); int dss_feat_get_num_ovls(void); diff --git a/drivers/video/fbdev/omap2/omapfb/dss/core.c b/drivers/video/fbdev/omap2/omapfb/dss/core.c index 29de4827589d..eecf695c16f4 100644 --- a/drivers/video/fbdev/omap2/omapfb/dss/core.c +++ b/drivers/video/fbdev/omap2/omapfb/dss/core.c @@ -206,8 +206,6 @@ static int __init omap_dss_probe(struct platform_device *pdev) if (def_disp_name) core.default_display_name = def_disp_name; - else if (pdata->default_display_name) - core.default_display_name = pdata->default_display_name; register_pm_notifier(&omap_dss_pm_notif_block); diff --git a/include/linux/platform_data/omapdss.h b/include/linux/platform_data/omapdss.h index 679177929045..7feb011ed500 100644 --- a/include/linux/platform_data/omapdss.h +++ b/include/linux/platform_data/omapdss.h @@ -27,7 +27,6 @@ enum omapdss_version { /* Board specific data */ struct omap_dss_board_info { - const char *default_display_name; int (*dsi_enable_pads)(int dsi_id, unsigned int lane_mask); void (*dsi_disable_pads)(int dsi_id, unsigned int lane_mask); int (*set_min_bus_tput)(struct device *dev, unsigned long r); -- cgit v1.2.3 From 466749f13e33d892cf9263d7efbc0ea713c23ed7 Mon Sep 17 00:00:00 2001 From: Thierry Reding Date: Mon, 10 Apr 2017 12:27:01 +0200 Subject: gpu: host1x: Flesh out kerneldoc Improve kerneldoc for the public parts of the host1x infrastructure in preparation for adding driver-specific part to the GPU documentation. Acked-by: Daniel Vetter Signed-off-by: Thierry Reding --- drivers/gpu/host1x/bus.c | 75 +++++++++++++++++++++++++++++++++++++++++ drivers/gpu/host1x/syncpt.c | 81 +++++++++++++++++++++++++++++++++++++++------ include/linux/host1x.h | 25 ++++++++++++++ 3 files changed, 170 insertions(+), 11 deletions(-) (limited to 'include/linux') diff --git a/drivers/gpu/host1x/bus.c b/drivers/gpu/host1x/bus.c index 561831e1ae2c..a048e3ac523d 100644 --- a/drivers/gpu/host1x/bus.c +++ b/drivers/gpu/host1x/bus.c @@ -40,6 +40,9 @@ struct host1x_subdev { /** * host1x_subdev_add() - add a new subdevice with an associated device node + * @device: host1x device to add the subdevice to + * @driver: host1x driver + * @np: device node */ static int host1x_subdev_add(struct host1x_device *device, struct device_node *np) @@ -62,6 +65,7 @@ static int host1x_subdev_add(struct host1x_device *device, /** * host1x_subdev_del() - remove subdevice + * @subdev: subdevice to remove */ static void host1x_subdev_del(struct host1x_subdev *subdev) { @@ -72,6 +76,8 @@ static void host1x_subdev_del(struct host1x_subdev *subdev) /** * host1x_device_parse_dt() - scan device tree and add matching subdevices + * @device: host1x logical device + * @driver: host1x driver */ static int host1x_device_parse_dt(struct host1x_device *device, struct host1x_driver *driver) @@ -166,6 +172,16 @@ static void host1x_subdev_unregister(struct host1x_device *device, mutex_unlock(&device->subdevs_lock); } +/** + * host1x_device_init() - initialize a host1x logical device + * @device: host1x logical device + * + * The driver for the host1x logical device can call this during execution of + * its &host1x_driver.probe implementation to initialize each of its clients. + * The client drivers access the subsystem specific driver data using the + * &host1x_client.parent field and driver data associated with it (usually by + * calling dev_get_drvdata()). + */ int host1x_device_init(struct host1x_device *device) { struct host1x_client *client; @@ -192,6 +208,15 @@ int host1x_device_init(struct host1x_device *device) } EXPORT_SYMBOL(host1x_device_init); +/** + * host1x_device_exit() - uninitialize host1x logical device + * @device: host1x logical device + * + * When the driver for a host1x logical device is unloaded, it can call this + * function to tear down each of its clients. Typically this is done after a + * subsystem-specific data structure is removed and the functionality can no + * longer be used. + */ int host1x_device_exit(struct host1x_device *device) { struct host1x_client *client; @@ -446,6 +471,14 @@ static void host1x_detach_driver(struct host1x *host1x, mutex_unlock(&host1x->devices_lock); } +/** + * host1x_register() - register a host1x controller + * @host1x: host1x controller + * + * The host1x controller driver uses this to register a host1x controller with + * the infrastructure. Note that all Tegra SoC generations have only ever come + * with a single host1x instance, so this function is somewhat academic. + */ int host1x_register(struct host1x *host1x) { struct host1x_driver *driver; @@ -464,6 +497,13 @@ int host1x_register(struct host1x *host1x) return 0; } +/** + * host1x_unregister() - unregister a host1x controller + * @host1x: host1x controller + * + * The host1x controller driver uses this to remove a host1x controller from + * the infrastructure. + */ int host1x_unregister(struct host1x *host1x) { struct host1x_driver *driver; @@ -513,6 +553,16 @@ static void host1x_device_shutdown(struct device *dev) driver->shutdown(device); } +/** + * host1x_driver_register_full() - register a host1x driver + * @driver: host1x driver + * @owner: owner module + * + * Drivers for host1x logical devices call this function to register a driver + * with the infrastructure. Note that since these drive logical devices, the + * registration of the driver actually triggers tho logical device creation. + * A logical device will be created for each host1x instance. + */ int host1x_driver_register_full(struct host1x_driver *driver, struct module *owner) { @@ -541,6 +591,13 @@ int host1x_driver_register_full(struct host1x_driver *driver, } EXPORT_SYMBOL(host1x_driver_register_full); +/** + * host1x_driver_unregister() - unregister a host1x driver + * @driver: host1x driver + * + * Unbinds the driver from each of the host1x logical devices that it is + * bound to, effectively removing the subsystem devices that they represent. + */ void host1x_driver_unregister(struct host1x_driver *driver) { driver_unregister(&driver->driver); @@ -551,6 +608,17 @@ void host1x_driver_unregister(struct host1x_driver *driver) } EXPORT_SYMBOL(host1x_driver_unregister); +/** + * host1x_client_register() - register a host1x client + * @client: host1x client + * + * Registers a host1x client with each host1x controller instance. Note that + * each client will only match their parent host1x controller and will only be + * associated with that instance. Once all clients have been registered with + * their parent host1x controller, the infrastructure will set up the logical + * device and call host1x_device_init(), which will in turn call each client's + * &host1x_client_ops.init implementation. + */ int host1x_client_register(struct host1x_client *client) { struct host1x *host1x; @@ -576,6 +644,13 @@ int host1x_client_register(struct host1x_client *client) } EXPORT_SYMBOL(host1x_client_register); +/** + * host1x_client_unregister() - unregister a host1x client + * @client: host1x client + * + * Removes a host1x client from its host1x controller instance. If a logical + * device has already been initialized, it will be torn down. + */ int host1x_client_unregister(struct host1x_client *client) { struct host1x_client *c; diff --git a/drivers/gpu/host1x/syncpt.c b/drivers/gpu/host1x/syncpt.c index 0ac026cdc30c..048ac9e344ce 100644 --- a/drivers/gpu/host1x/syncpt.c +++ b/drivers/gpu/host1x/syncpt.c @@ -99,14 +99,24 @@ unlock: return NULL; } +/** + * host1x_syncpt_id() - retrieve syncpoint ID + * @sp: host1x syncpoint + * + * Given a pointer to a struct host1x_syncpt, retrieves its ID. This ID is + * often used as a value to program into registers that control how hardware + * blocks interact with syncpoints. + */ u32 host1x_syncpt_id(struct host1x_syncpt *sp) { return sp->id; } EXPORT_SYMBOL(host1x_syncpt_id); -/* - * Updates the value sent to hardware. +/** + * host1x_syncpt_incr_max() - update the value sent to hardware + * @sp: host1x syncpoint + * @incrs: number of increments */ u32 host1x_syncpt_incr_max(struct host1x_syncpt *sp, u32 incrs) { @@ -175,8 +185,9 @@ u32 host1x_syncpt_load_wait_base(struct host1x_syncpt *sp) return sp->base_val; } -/* - * Increment syncpoint value from cpu, updating cache +/** + * host1x_syncpt_incr() - increment syncpoint value from CPU, updating cache + * @sp: host1x syncpoint */ int host1x_syncpt_incr(struct host1x_syncpt *sp) { @@ -195,8 +206,12 @@ static bool syncpt_load_min_is_expired(struct host1x_syncpt *sp, u32 thresh) return host1x_syncpt_is_expired(sp, thresh); } -/* - * Main entrypoint for syncpoint value waits. +/** + * host1x_syncpt_wait() - wait for a syncpoint to reach a given value + * @sp: host1x syncpoint + * @thresh: threshold + * @timeout: maximum time to wait for the syncpoint to reach the given value + * @value: return location for the syncpoint value */ int host1x_syncpt_wait(struct host1x_syncpt *sp, u32 thresh, long timeout, u32 *value) @@ -402,6 +417,16 @@ int host1x_syncpt_init(struct host1x *host) return 0; } +/** + * host1x_syncpt_request() - request a syncpoint + * @dev: device requesting the syncpoint + * @flags: flags + * + * host1x client drivers can use this function to allocate a syncpoint for + * subsequent use. A syncpoint returned by this function will be reserved for + * use by the client exclusively. When no longer using a syncpoint, a host1x + * client driver needs to release it using host1x_syncpt_free(). + */ struct host1x_syncpt *host1x_syncpt_request(struct device *dev, unsigned long flags) { @@ -411,6 +436,16 @@ struct host1x_syncpt *host1x_syncpt_request(struct device *dev, } EXPORT_SYMBOL(host1x_syncpt_request); +/** + * host1x_syncpt_free() - free a requested syncpoint + * @sp: host1x syncpoint + * + * Release a syncpoint previously allocated using host1x_syncpt_request(). A + * host1x client driver should call this when the syncpoint is no longer in + * use. Note that client drivers must ensure that the syncpoint doesn't remain + * under the control of hardware after calling this function, otherwise two + * clients may end up trying to access the same syncpoint concurrently. + */ void host1x_syncpt_free(struct host1x_syncpt *sp) { if (!sp) @@ -438,9 +473,12 @@ void host1x_syncpt_deinit(struct host1x *host) kfree(sp->name); } -/* - * Read max. It indicates how many operations there are in queue, either in - * channel or in a software thread. +/** + * host1x_syncpt_read_max() - read maximum syncpoint value + * @sp: host1x syncpoint + * + * The maximum syncpoint value indicates how many operations there are in + * queue, either in channel or in a software thread. */ u32 host1x_syncpt_read_max(struct host1x_syncpt *sp) { @@ -450,8 +488,12 @@ u32 host1x_syncpt_read_max(struct host1x_syncpt *sp) } EXPORT_SYMBOL(host1x_syncpt_read_max); -/* - * Read min, which is a shadow of the current sync point value in hardware. +/** + * host1x_syncpt_read_min() - read minimum syncpoint value + * @sp: host1x syncpoint + * + * The minimum syncpoint value is a shadow of the current sync point value in + * hardware. */ u32 host1x_syncpt_read_min(struct host1x_syncpt *sp) { @@ -461,6 +503,10 @@ u32 host1x_syncpt_read_min(struct host1x_syncpt *sp) } EXPORT_SYMBOL(host1x_syncpt_read_min); +/** + * host1x_syncpt_read() - read the current syncpoint value + * @sp: host1x syncpoint + */ u32 host1x_syncpt_read(struct host1x_syncpt *sp) { return host1x_syncpt_load(sp); @@ -482,6 +528,11 @@ unsigned int host1x_syncpt_nb_mlocks(struct host1x *host) return host->info->nb_mlocks; } +/** + * host1x_syncpt_get() - obtain a syncpoint by ID + * @host: host1x controller + * @id: syncpoint ID + */ struct host1x_syncpt *host1x_syncpt_get(struct host1x *host, unsigned int id) { if (id >= host->info->nb_pts) @@ -491,12 +542,20 @@ struct host1x_syncpt *host1x_syncpt_get(struct host1x *host, unsigned int id) } EXPORT_SYMBOL(host1x_syncpt_get); +/** + * host1x_syncpt_get_base() - obtain the wait base associated with a syncpoint + * @sp: host1x syncpoint + */ struct host1x_syncpt_base *host1x_syncpt_get_base(struct host1x_syncpt *sp) { return sp ? sp->base : NULL; } EXPORT_SYMBOL(host1x_syncpt_get_base); +/** + * host1x_syncpt_base_id() - retrieve the ID of a syncpoint wait base + * @base: host1x syncpoint wait base + */ u32 host1x_syncpt_base_id(struct host1x_syncpt_base *base) { return base->id; diff --git a/include/linux/host1x.h b/include/linux/host1x.h index 3d04aa1dc83e..840a8ad627b2 100644 --- a/include/linux/host1x.h +++ b/include/linux/host1x.h @@ -32,11 +32,27 @@ enum host1x_class { struct host1x_client; +/** + * struct host1x_client_ops - host1x client operations + * @init: host1x client initialization code + * @exit: host1x client tear down code + */ struct host1x_client_ops { int (*init)(struct host1x_client *client); int (*exit)(struct host1x_client *client); }; +/** + * struct host1x_client - host1x client structure + * @list: list node for the host1x client + * @parent: pointer to struct device representing the host1x controller + * @dev: pointer to struct device backing this host1x client + * @ops: host1x client operations + * @class: host1x class represented by this client + * @channel: host1x channel associated with this client + * @syncpts: array of syncpoints requested for this client + * @num_syncpts: number of syncpoints requested for this client + */ struct host1x_client { struct list_head list; struct device *parent; @@ -251,6 +267,15 @@ void host1x_job_unpin(struct host1x_job *job); struct host1x_device; +/** + * struct host1x_driver - host1x logical device driver + * @driver: core driver + * @subdevs: table of OF device IDs matching subdevices for this driver + * @list: list node for the driver + * @probe: called when the host1x logical device is probed + * @remove: called when the host1x logical device is removed + * @shutdown: called when the host1x logical device is shut down + */ struct host1x_driver { struct device_driver driver; -- cgit v1.2.3 From d0fbbdff2e19aabccc1107b7e12ab9f3cbf626ef Mon Sep 17 00:00:00 2001 From: Dmitry Osipenko Date: Thu, 15 Jun 2017 02:18:27 +0300 Subject: drm/tegra: Correct copying of waitchecks and disable them in the 'submit' IOCTL The waitchecks along with multiple syncpoints per submit are not ready for use yet, let's forbid them for now. Signed-off-by: Dmitry Osipenko Reviewed-by: Mikko Perttunen Signed-off-by: Thierry Reding --- drivers/gpu/drm/tegra/drm.c | 60 ++++++++++++++++++++++++++++++++++++++++++--- drivers/gpu/host1x/job.h | 7 ------ include/linux/host1x.h | 7 ++++++ 3 files changed, 63 insertions(+), 11 deletions(-) (limited to 'include/linux') diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c index 51e20e015053..0928f2bb4203 100644 --- a/drivers/gpu/drm/tegra/drm.c +++ b/drivers/gpu/drm/tegra/drm.c @@ -349,6 +349,36 @@ static int host1x_reloc_copy_from_user(struct host1x_reloc *dest, return 0; } +static int host1x_waitchk_copy_from_user(struct host1x_waitchk *dest, + struct drm_tegra_waitchk __user *src, + struct drm_file *file) +{ + u32 cmdbuf; + int err; + + err = get_user(cmdbuf, &src->handle); + if (err < 0) + return err; + + err = get_user(dest->offset, &src->offset); + if (err < 0) + return err; + + err = get_user(dest->syncpt_id, &src->syncpt); + if (err < 0) + return err; + + err = get_user(dest->thresh, &src->thresh); + if (err < 0) + return err; + + dest->bo = host1x_bo_lookup(file, cmdbuf); + if (!dest->bo) + return -ENOENT; + + return 0; +} + int tegra_drm_submit(struct tegra_drm_context *context, struct drm_tegra_submit *args, struct drm_device *drm, struct drm_file *file) @@ -370,6 +400,10 @@ int tegra_drm_submit(struct tegra_drm_context *context, if (args->num_syncpts != 1) return -EINVAL; + /* We don't yet support waitchks */ + if (args->num_waitchks != 0) + return -EINVAL; + job = host1x_job_alloc(context->channel, args->num_cmdbufs, args->num_relocs, args->num_waitchks); if (!job) @@ -458,10 +492,28 @@ int tegra_drm_submit(struct tegra_drm_context *context, } } - if (copy_from_user(job->waitchk, waitchks, - sizeof(*waitchks) * num_waitchks)) { - err = -EFAULT; - goto fail; + /* copy and resolve waitchks from submit */ + while (num_waitchks--) { + struct host1x_waitchk *wait = &job->waitchk[num_waitchks]; + struct tegra_bo *obj; + + err = host1x_waitchk_copy_from_user(wait, + &waitchks[num_waitchks], + file); + if (err < 0) + goto fail; + + obj = host1x_to_tegra_bo(wait->bo); + + /* + * The unaligned offset will cause an unaligned write during + * of the waitchks patching, corrupting the commands stream. + */ + if (wait->offset & 3 || + wait->offset >= obj->gem.size) { + err = -EINVAL; + goto fail; + } } if (copy_from_user(&syncpt, (void __user *)(uintptr_t)args->syncpts, diff --git a/drivers/gpu/host1x/job.h b/drivers/gpu/host1x/job.h index 878239c476d2..0debd93a1849 100644 --- a/drivers/gpu/host1x/job.h +++ b/drivers/gpu/host1x/job.h @@ -34,13 +34,6 @@ struct host1x_cmdbuf { u32 pad; }; -struct host1x_waitchk { - struct host1x_bo *bo; - u32 offset; - u32 syncpt_id; - u32 thresh; -}; - struct host1x_job_unpin_data { struct host1x_bo *bo; struct sg_table *sgt; diff --git a/include/linux/host1x.h b/include/linux/host1x.h index 840a8ad627b2..ba0b245da732 100644 --- a/include/linux/host1x.h +++ b/include/linux/host1x.h @@ -193,6 +193,13 @@ struct host1x_reloc { unsigned long shift; }; +struct host1x_waitchk { + struct host1x_bo *bo; + u32 offset; + u32 syncpt_id; + u32 thresh; +}; + struct host1x_job { /* When refcount goes to zero, job can be freed */ struct kref ref; -- cgit v1.2.3 From 0f563a4bf66e5182f0882efee398f7e6bc0bb1be Mon Sep 17 00:00:00 2001 From: Dmitry Osipenko Date: Thu, 15 Jun 2017 02:18:37 +0300 Subject: gpu: host1x: Forbid unrelated SETCLASS opcode in the firewall Several channels could be made to write the same unit concurrently via the SETCLASS opcode, trusting userspace is a bad idea. It should be possible to drop the per-client channel reservation and add a per-unit locking by inserting MLOCK's to the command stream to re-allow the SETCLASS opcode, but it will be much more work. Let's forbid the unit-unrelated class changes for now. Signed-off-by: Dmitry Osipenko Reviewed-by: Erik Faye-Lund Reviewed-by: Mikko Perttunen Signed-off-by: Thierry Reding --- drivers/gpu/drm/tegra/drm.c | 1 + drivers/gpu/drm/tegra/drm.h | 1 + drivers/gpu/drm/tegra/gr2d.c | 7 +++++++ drivers/gpu/host1x/job.c | 24 ++++++++++++++++++++---- include/linux/host1x.h | 3 +++ 5 files changed, 32 insertions(+), 4 deletions(-) (limited to 'include/linux') diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c index b44f1eddb570..4a46ba846a0f 100644 --- a/drivers/gpu/drm/tegra/drm.c +++ b/drivers/gpu/drm/tegra/drm.c @@ -532,6 +532,7 @@ int tegra_drm_submit(struct tegra_drm_context *context, } job->is_addr_reg = context->client->ops->is_addr_reg; + job->is_valid_class = context->client->ops->is_valid_class; job->syncpt_incrs = syncpt.incrs; job->syncpt_id = syncpt.id; job->timeout = 10000; diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h index 85aa2e3d9d4e..6d6da01282f3 100644 --- a/drivers/gpu/drm/tegra/drm.h +++ b/drivers/gpu/drm/tegra/drm.h @@ -83,6 +83,7 @@ struct tegra_drm_client_ops { struct tegra_drm_context *context); void (*close_channel)(struct tegra_drm_context *context); int (*is_addr_reg)(struct device *dev, u32 class, u32 offset); + int (*is_valid_class)(u32 class); int (*submit)(struct tegra_drm_context *context, struct drm_tegra_submit *args, struct drm_device *drm, struct drm_file *file); diff --git a/drivers/gpu/drm/tegra/gr2d.c b/drivers/gpu/drm/tegra/gr2d.c index 02cd3e37a6ec..fbe0b8b25b42 100644 --- a/drivers/gpu/drm/tegra/gr2d.c +++ b/drivers/gpu/drm/tegra/gr2d.c @@ -109,10 +109,17 @@ static int gr2d_is_addr_reg(struct device *dev, u32 class, u32 offset) return 0; } +static int gr2d_is_valid_class(u32 class) +{ + return (class == HOST1X_CLASS_GR2D || + class == HOST1X_CLASS_GR2D_SB); +} + static const struct tegra_drm_client_ops gr2d_ops = { .open_channel = gr2d_open_channel, .close_channel = gr2d_close_channel, .is_addr_reg = gr2d_is_addr_reg, + .is_valid_class = gr2d_is_valid_class, .submit = tegra_drm_submit, }; diff --git a/drivers/gpu/host1x/job.c b/drivers/gpu/host1x/job.c index 54230ec4f81e..ef746f7afb88 100644 --- a/drivers/gpu/host1x/job.c +++ b/drivers/gpu/host1x/job.c @@ -356,6 +356,9 @@ struct host1x_firewall { static int check_register(struct host1x_firewall *fw, unsigned long offset) { + if (!fw->job->is_addr_reg) + return 0; + if (fw->job->is_addr_reg(fw->dev, fw->class, offset)) { if (!fw->num_relocs) return -EINVAL; @@ -370,6 +373,19 @@ static int check_register(struct host1x_firewall *fw, unsigned long offset) return 0; } +static int check_class(struct host1x_firewall *fw, u32 class) +{ + if (!fw->job->is_valid_class) { + if (fw->class != class) + return -EINVAL; + } else { + if (!fw->job->is_valid_class(fw->class)) + return -EINVAL; + } + + return 0; +} + static int check_mask(struct host1x_firewall *fw) { u32 mask = fw->mask; @@ -443,11 +459,9 @@ static int validate(struct host1x_firewall *fw, struct host1x_job_gather *g) { u32 *cmdbuf_base = (u32 *)fw->job->gather_copy_mapped + (g->offset / sizeof(u32)); + u32 job_class = fw->class; int err = 0; - if (!fw->job->is_addr_reg) - return 0; - fw->words = g->words; fw->cmdbuf = g->bo; fw->offset = 0; @@ -467,7 +481,9 @@ static int validate(struct host1x_firewall *fw, struct host1x_job_gather *g) fw->class = word >> 6 & 0x3ff; fw->mask = word & 0x3f; fw->reg = word >> 16 & 0xfff; - err = check_mask(fw); + err = check_class(fw, job_class); + if (!err) + err = check_mask(fw); if (err) goto out; break; diff --git a/include/linux/host1x.h b/include/linux/host1x.h index ba0b245da732..b5358f855d9e 100644 --- a/include/linux/host1x.h +++ b/include/linux/host1x.h @@ -251,6 +251,9 @@ struct host1x_job { /* Check if register is marked as an address reg */ int (*is_addr_reg)(struct device *dev, u32 reg, u32 class); + /* Check if class belongs to the unit */ + int (*is_valid_class)(u32 class); + /* Request a SETCLASS to this class */ u32 class; -- cgit v1.2.3 From a2b78b0d53f0808ebc2a0368b589a5cb6b672294 Mon Sep 17 00:00:00 2001 From: Dmitry Osipenko Date: Thu, 15 Jun 2017 02:18:38 +0300 Subject: gpu: host1x: Correct swapped arguments in the is_addr_reg() definition Arguments of the .is_addr_reg() are swapped in the definition of the function, that is quite confusing. Signed-off-by: Dmitry Osipenko Reviewed-by: Erik Faye-Lund Reviewed-by: Mikko Perttunen Signed-off-by: Thierry Reding --- include/linux/host1x.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'include/linux') diff --git a/include/linux/host1x.h b/include/linux/host1x.h index b5358f855d9e..476da0e06bb2 100644 --- a/include/linux/host1x.h +++ b/include/linux/host1x.h @@ -249,7 +249,7 @@ struct host1x_job { u8 *gather_copy_mapped; /* Check if register is marked as an address reg */ - int (*is_addr_reg)(struct device *dev, u32 reg, u32 class); + int (*is_addr_reg)(struct device *dev, u32 class, u32 reg); /* Check if class belongs to the unit */ int (*is_valid_class)(u32 class); -- cgit v1.2.3 From 8474b02531c4881a762c52ef869c52429e38633f Mon Sep 17 00:00:00 2001 From: Mikko Perttunen Date: Thu, 15 Jun 2017 02:18:42 +0300 Subject: gpu: host1x: Refactor channel allocation code This is largely a rewrite of the Host1x channel allocation code, bringing several changes: - The previous code could deadlock due to an interaction between the 'reflock' mutex and CDMA timeout handling. This gets rid of the mutex. - Support for more than 32 channels, required for Tegra186 - General refactoring, including better encapsulation of channel ownership handling into channel.c Signed-off-by: Mikko Perttunen Reviewed-by: Dmitry Osipenko Tested-by: Dmitry Osipenko Signed-off-by: Thierry Reding --- drivers/gpu/drm/tegra/gr2d.c | 4 +- drivers/gpu/drm/tegra/gr3d.c | 4 +- drivers/gpu/drm/tegra/vic.c | 4 +- drivers/gpu/host1x/channel.c | 147 +++++++++++++++++++++++-------------- drivers/gpu/host1x/channel.h | 21 ++++-- drivers/gpu/host1x/debug.c | 47 +++++------- drivers/gpu/host1x/dev.c | 7 +- drivers/gpu/host1x/dev.h | 6 +- drivers/gpu/host1x/hw/channel_hw.c | 4 - include/linux/host1x.h | 1 - 10 files changed, 135 insertions(+), 110 deletions(-) (limited to 'include/linux') diff --git a/drivers/gpu/drm/tegra/gr2d.c b/drivers/gpu/drm/tegra/gr2d.c index fbe0b8b25b42..6ea070da7718 100644 --- a/drivers/gpu/drm/tegra/gr2d.c +++ b/drivers/gpu/drm/tegra/gr2d.c @@ -38,7 +38,7 @@ static int gr2d_init(struct host1x_client *client) client->syncpts[0] = host1x_syncpt_request(client->dev, flags); if (!client->syncpts[0]) { - host1x_channel_free(gr2d->channel); + host1x_channel_put(gr2d->channel); return -ENOMEM; } @@ -57,7 +57,7 @@ static int gr2d_exit(struct host1x_client *client) return err; host1x_syncpt_free(client->syncpts[0]); - host1x_channel_free(gr2d->channel); + host1x_channel_put(gr2d->channel); return 0; } diff --git a/drivers/gpu/drm/tegra/gr3d.c b/drivers/gpu/drm/tegra/gr3d.c index 13f0d1b7cd98..cee2ab645cde 100644 --- a/drivers/gpu/drm/tegra/gr3d.c +++ b/drivers/gpu/drm/tegra/gr3d.c @@ -48,7 +48,7 @@ static int gr3d_init(struct host1x_client *client) client->syncpts[0] = host1x_syncpt_request(client->dev, flags); if (!client->syncpts[0]) { - host1x_channel_free(gr3d->channel); + host1x_channel_put(gr3d->channel); return -ENOMEM; } @@ -67,7 +67,7 @@ static int gr3d_exit(struct host1x_client *client) return err; host1x_syncpt_free(client->syncpts[0]); - host1x_channel_free(gr3d->channel); + host1x_channel_put(gr3d->channel); return 0; } diff --git a/drivers/gpu/drm/tegra/vic.c b/drivers/gpu/drm/tegra/vic.c index cd804e404a11..47cb1aaa58b1 100644 --- a/drivers/gpu/drm/tegra/vic.c +++ b/drivers/gpu/drm/tegra/vic.c @@ -182,7 +182,7 @@ static int vic_init(struct host1x_client *client) free_syncpt: host1x_syncpt_free(client->syncpts[0]); free_channel: - host1x_channel_free(vic->channel); + host1x_channel_put(vic->channel); detach_device: if (tegra->domain) iommu_detach_device(tegra->domain, vic->dev); @@ -203,7 +203,7 @@ static int vic_exit(struct host1x_client *client) return err; host1x_syncpt_free(client->syncpts[0]); - host1x_channel_free(vic->channel); + host1x_channel_put(vic->channel); if (vic->domain) { iommu_detach_device(vic->domain, vic->dev); diff --git a/drivers/gpu/host1x/channel.c b/drivers/gpu/host1x/channel.c index 8f437d924c10..db9b91d1384c 100644 --- a/drivers/gpu/host1x/channel.c +++ b/drivers/gpu/host1x/channel.c @@ -24,19 +24,33 @@ #include "job.h" /* Constructor for the host1x device list */ -int host1x_channel_list_init(struct host1x *host) +int host1x_channel_list_init(struct host1x_channel_list *chlist, + unsigned int num_channels) { - INIT_LIST_HEAD(&host->chlist.list); - mutex_init(&host->chlist_mutex); - - if (host->info->nb_channels > BITS_PER_LONG) { - WARN(1, "host1x hardware has more channels than supported by the driver\n"); - return -ENOSYS; + chlist->channels = kcalloc(num_channels, sizeof(struct host1x_channel), + GFP_KERNEL); + if (!chlist->channels) + return -ENOMEM; + + chlist->allocated_channels = + kcalloc(BITS_TO_LONGS(num_channels), sizeof(unsigned long), + GFP_KERNEL); + if (!chlist->allocated_channels) { + kfree(chlist->channels); + return -ENOMEM; } + bitmap_zero(chlist->allocated_channels, num_channels); + return 0; } +void host1x_channel_list_free(struct host1x_channel_list *chlist) +{ + kfree(chlist->allocated_channels); + kfree(chlist->channels); +} + int host1x_job_submit(struct host1x_job *job) { struct host1x *host = dev_get_drvdata(job->channel->dev->parent); @@ -47,86 +61,107 @@ EXPORT_SYMBOL(host1x_job_submit); struct host1x_channel *host1x_channel_get(struct host1x_channel *channel) { - int err = 0; + kref_get(&channel->refcount); - mutex_lock(&channel->reflock); + return channel; +} +EXPORT_SYMBOL(host1x_channel_get); - if (channel->refcount == 0) - err = host1x_cdma_init(&channel->cdma); +/** + * host1x_channel_get_index() - Attempt to get channel reference by index + * @host: Host1x device object + * @index: Index of channel + * + * If channel number @index is currently allocated, increase its refcount + * and return a pointer to it. Otherwise, return NULL. + */ +struct host1x_channel *host1x_channel_get_index(struct host1x *host, + unsigned int index) +{ + struct host1x_channel *ch = &host->channel_list.channels[index]; - if (!err) - channel->refcount++; + if (!kref_get_unless_zero(&ch->refcount)) + return NULL; - mutex_unlock(&channel->reflock); + return ch; +} + +static void release_channel(struct kref *kref) +{ + struct host1x_channel *channel = + container_of(kref, struct host1x_channel, refcount); + struct host1x *host = dev_get_drvdata(channel->dev->parent); + struct host1x_channel_list *chlist = &host->channel_list; + + host1x_hw_cdma_stop(host, &channel->cdma); + host1x_cdma_deinit(&channel->cdma); - return err ? NULL : channel; + clear_bit(channel->id, chlist->allocated_channels); } -EXPORT_SYMBOL(host1x_channel_get); void host1x_channel_put(struct host1x_channel *channel) { - mutex_lock(&channel->reflock); + kref_put(&channel->refcount, release_channel); +} +EXPORT_SYMBOL(host1x_channel_put); - if (channel->refcount == 1) { - struct host1x *host = dev_get_drvdata(channel->dev->parent); +static struct host1x_channel *acquire_unused_channel(struct host1x *host) +{ + struct host1x_channel_list *chlist = &host->channel_list; + unsigned int max_channels = host->info->nb_channels; + unsigned int index; - host1x_hw_cdma_stop(host, &channel->cdma); - host1x_cdma_deinit(&channel->cdma); + index = find_first_zero_bit(chlist->allocated_channels, max_channels); + if (index >= max_channels) { + dev_err(host->dev, "failed to find free channel\n"); + return NULL; } - channel->refcount--; + chlist->channels[index].id = index; - mutex_unlock(&channel->reflock); + set_bit(index, chlist->allocated_channels); + + return &chlist->channels[index]; } -EXPORT_SYMBOL(host1x_channel_put); +/** + * host1x_channel_request() - Allocate a channel + * @device: Host1x unit this channel will be used to send commands to + * + * Allocates a new host1x channel for @device. If there are no free channels, + * this will sleep until one becomes available. May return NULL if CDMA + * initialization fails. + */ struct host1x_channel *host1x_channel_request(struct device *dev) { struct host1x *host = dev_get_drvdata(dev->parent); - unsigned int max_channels = host->info->nb_channels; - struct host1x_channel *channel = NULL; - unsigned long index; + struct host1x_channel_list *chlist = &host->channel_list; + struct host1x_channel *channel; int err; - mutex_lock(&host->chlist_mutex); + channel = acquire_unused_channel(host); + if (!channel) + return NULL; - index = find_first_zero_bit(&host->allocated_channels, max_channels); - if (index >= max_channels) - goto fail; + kref_init(&channel->refcount); + mutex_init(&channel->submitlock); + channel->dev = dev; - channel = kzalloc(sizeof(*channel), GFP_KERNEL); - if (!channel) + err = host1x_hw_channel_init(host, channel, channel->id); + if (err < 0) goto fail; - err = host1x_hw_channel_init(host, channel, index); + err = host1x_cdma_init(&channel->cdma); if (err < 0) goto fail; - /* Link device to host1x_channel */ - channel->dev = dev; - - /* Add to channel list */ - list_add_tail(&channel->list, &host->chlist.list); - - host->allocated_channels |= BIT(index); - - mutex_unlock(&host->chlist_mutex); return channel; fail: - dev_err(dev, "failed to init channel\n"); - kfree(channel); - mutex_unlock(&host->chlist_mutex); - return NULL; -} -EXPORT_SYMBOL(host1x_channel_request); + clear_bit(channel->id, chlist->allocated_channels); -void host1x_channel_free(struct host1x_channel *channel) -{ - struct host1x *host = dev_get_drvdata(channel->dev->parent); + dev_err(dev, "failed to initialize channel\n"); - host->allocated_channels &= ~BIT(channel->id); - list_del(&channel->list); - kfree(channel); + return NULL; } -EXPORT_SYMBOL(host1x_channel_free); +EXPORT_SYMBOL(host1x_channel_request); diff --git a/drivers/gpu/host1x/channel.h b/drivers/gpu/host1x/channel.h index df767cf90d51..7068e42d42df 100644 --- a/drivers/gpu/host1x/channel.h +++ b/drivers/gpu/host1x/channel.h @@ -20,17 +20,21 @@ #define __HOST1X_CHANNEL_H #include +#include #include "cdma.h" struct host1x; +struct host1x_channel; -struct host1x_channel { - struct list_head list; +struct host1x_channel_list { + struct host1x_channel *channels; + unsigned long *allocated_channels; +}; - unsigned int refcount; +struct host1x_channel { + struct kref refcount; unsigned int id; - struct mutex reflock; struct mutex submitlock; void __iomem *regs; struct device *dev; @@ -38,9 +42,10 @@ struct host1x_channel { }; /* channel list operations */ -int host1x_channel_list_init(struct host1x *host); - -#define host1x_for_each_channel(host, channel) \ - list_for_each_entry(channel, &host->chlist.list, list) +int host1x_channel_list_init(struct host1x_channel_list *chlist, + unsigned int num_channels); +void host1x_channel_list_free(struct host1x_channel_list *chlist); +struct host1x_channel *host1x_channel_get_index(struct host1x *host, + unsigned int index); #endif diff --git a/drivers/gpu/host1x/debug.c b/drivers/gpu/host1x/debug.c index d9330fcc62ad..2aae0e63214c 100644 --- a/drivers/gpu/host1x/debug.c +++ b/drivers/gpu/host1x/debug.c @@ -43,24 +43,19 @@ void host1x_debug_output(struct output *o, const char *fmt, ...) o->fn(o->ctx, o->buf, len); } -static int show_channels(struct host1x_channel *ch, void *data, bool show_fifo) +static int show_channel(struct host1x_channel *ch, void *data, bool show_fifo) { struct host1x *m = dev_get_drvdata(ch->dev->parent); struct output *o = data; - mutex_lock(&ch->reflock); + mutex_lock(&ch->cdma.lock); - if (ch->refcount) { - mutex_lock(&ch->cdma.lock); + if (show_fifo) + host1x_hw_show_channel_fifo(m, ch, o); - if (show_fifo) - host1x_hw_show_channel_fifo(m, ch, o); + host1x_hw_show_channel_cdma(m, ch, o); - host1x_hw_show_channel_cdma(m, ch, o); - mutex_unlock(&ch->cdma.lock); - } - - mutex_unlock(&ch->reflock); + mutex_unlock(&ch->cdma.lock); return 0; } @@ -94,28 +89,22 @@ static void show_syncpts(struct host1x *m, struct output *o) host1x_debug_output(o, "\n"); } -static void show_all(struct host1x *m, struct output *o) +static void show_all(struct host1x *m, struct output *o, bool show_fifo) { - struct host1x_channel *ch; + int i; host1x_hw_show_mlocks(m, o); show_syncpts(m, o); host1x_debug_output(o, "---- channels ----\n"); - host1x_for_each_channel(m, ch) - show_channels(ch, o, true); -} - -static void show_all_no_fifo(struct host1x *host1x, struct output *o) -{ - struct host1x_channel *ch; - - host1x_hw_show_mlocks(host1x, o); - show_syncpts(host1x, o); - host1x_debug_output(o, "---- channels ----\n"); + for (i = 0; i < m->info->nb_channels; ++i) { + struct host1x_channel *ch = host1x_channel_get_index(m, i); - host1x_for_each_channel(host1x, ch) - show_channels(ch, o, false); + if (ch) { + show_channel(ch, o, show_fifo); + host1x_channel_put(ch); + } + } } static int host1x_debug_show_all(struct seq_file *s, void *unused) @@ -125,7 +114,7 @@ static int host1x_debug_show_all(struct seq_file *s, void *unused) .ctx = s }; - show_all(s->private, &o); + show_all(s->private, &o, true); return 0; } @@ -137,7 +126,7 @@ static int host1x_debug_show(struct seq_file *s, void *unused) .ctx = s }; - show_all_no_fifo(s->private, &o); + show_all(s->private, &o, false); return 0; } @@ -216,7 +205,7 @@ void host1x_debug_dump(struct host1x *host1x) .fn = write_to_printk }; - show_all(host1x, &o); + show_all(host1x, &o, true); } void host1x_debug_dump_syncpts(struct host1x *host1x) diff --git a/drivers/gpu/host1x/dev.c b/drivers/gpu/host1x/dev.c index f05ebb14fa63..5c1c711a21af 100644 --- a/drivers/gpu/host1x/dev.c +++ b/drivers/gpu/host1x/dev.c @@ -198,7 +198,8 @@ static int host1x_probe(struct platform_device *pdev) host->iova_end = geometry->aperture_end; } - err = host1x_channel_list_init(host); + err = host1x_channel_list_init(&host->channel_list, + host->info->nb_channels); if (err) { dev_err(&pdev->dev, "failed to initialize channel list\n"); goto fail_detach_device; @@ -207,7 +208,7 @@ static int host1x_probe(struct platform_device *pdev) err = clk_prepare_enable(host->clk); if (err < 0) { dev_err(&pdev->dev, "failed to enable clock\n"); - goto fail_detach_device; + goto fail_free_channels; } err = reset_control_deassert(host->rst); @@ -244,6 +245,8 @@ fail_reset_assert: reset_control_assert(host->rst); fail_unprepare_disable: clk_disable_unprepare(host->clk); +fail_free_channels: + host1x_channel_list_free(&host->channel_list); fail_detach_device: if (host->domain) { put_iova_domain(&host->iova); diff --git a/drivers/gpu/host1x/dev.h b/drivers/gpu/host1x/dev.h index 229d08b6a45e..ffdbc15b749b 100644 --- a/drivers/gpu/host1x/dev.h +++ b/drivers/gpu/host1x/dev.h @@ -129,10 +129,8 @@ struct host1x { struct host1x_syncpt *nop_sp; struct mutex syncpt_mutex; - struct mutex chlist_mutex; - struct host1x_channel chlist; - unsigned long allocated_channels; - unsigned int num_allocated_channels; + + struct host1x_channel_list channel_list; struct dentry *debugfs; diff --git a/drivers/gpu/host1x/hw/channel_hw.c b/drivers/gpu/host1x/hw/channel_hw.c index 5e8df78b7acd..8447a56c41ca 100644 --- a/drivers/gpu/host1x/hw/channel_hw.c +++ b/drivers/gpu/host1x/hw/channel_hw.c @@ -181,10 +181,6 @@ error: static int host1x_channel_init(struct host1x_channel *ch, struct host1x *dev, unsigned int index) { - ch->id = index; - mutex_init(&ch->reflock); - mutex_init(&ch->submitlock); - ch->regs = dev->regs + index * HOST1X_CHANNEL_SIZE; return 0; } diff --git a/include/linux/host1x.h b/include/linux/host1x.h index 476da0e06bb2..630b1a98ab58 100644 --- a/include/linux/host1x.h +++ b/include/linux/host1x.h @@ -172,7 +172,6 @@ struct host1x_channel; struct host1x_job; struct host1x_channel *host1x_channel_request(struct device *dev); -void host1x_channel_free(struct host1x_channel *channel); struct host1x_channel *host1x_channel_get(struct host1x_channel *channel); void host1x_channel_put(struct host1x_channel *channel); int host1x_job_submit(struct host1x_job *job); -- cgit v1.2.3