From c0f83d164fb8f3a2b7bc379a6c1e27d1123a9eab Mon Sep 17 00:00:00 2001 From: Marek Szyprowski Date: Fri, 27 Mar 2020 17:21:26 +0100 Subject: [PATCH 1/4] drm/prime: fix extracting of the DMA addresses from a scatterlist Scatterlist elements contains both pages and DMA addresses, but one should not assume 1:1 relation between them. The sg->length is the size of the physical memory chunk described by the sg->page, while sg_dma_len(sg) is the size of the DMA (IO virtual) chunk described by the sg_dma_address(sg). The proper way of extracting both: pages and DMA addresses of the whole buffer described by a scatterlist it to iterate independently over the sg->pages/sg->length and sg_dma_address(sg)/sg_dma_len(sg) entries. Fixes: 42e67b479eab ("drm/prime: use dma length macro when mapping sg") Signed-off-by: Marek Szyprowski Reviewed-by: Alex Deucher Signed-off-by: Alex Deucher Link: https://patchwork.freedesktop.org/patch/msgid/20200327162126.29705-1-m.szyprowski@samsung.com Cc: stable@vger.kernel.org --- drivers/gpu/drm/drm_prime.c | 37 +++++++++++++++++++++++++------------ 1 file changed, 25 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index 1de2cde2277c..282774e469ac 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -962,27 +962,40 @@ int drm_prime_sg_to_page_addr_arrays(struct sg_table *sgt, struct page **pages, unsigned count; struct scatterlist *sg; struct page *page; - u32 len, index; + u32 page_len, page_index; dma_addr_t addr; + u32 dma_len, dma_index; - index = 0; + /* + * Scatterlist elements contains both pages and DMA addresses, but + * one shoud not assume 1:1 relation between them. The sg->length is + * the size of the physical memory chunk described by the sg->page, + * while sg_dma_len(sg) is the size of the DMA (IO virtual) chunk + * described by the sg_dma_address(sg). + */ + page_index = 0; + dma_index = 0; for_each_sg(sgt->sgl, sg, sgt->nents, count) { - len = sg_dma_len(sg); + page_len = sg->length; page = sg_page(sg); + dma_len = sg_dma_len(sg); addr = sg_dma_address(sg); - while (len > 0) { - if (WARN_ON(index >= max_entries)) + while (pages && page_len > 0) { + if (WARN_ON(page_index >= max_entries)) return -1; - if (pages) - pages[index] = page; - if (addrs) - addrs[index] = addr; - + pages[page_index] = page; page++; + page_len -= PAGE_SIZE; + page_index++; + } + while (addrs && dma_len > 0) { + if (WARN_ON(dma_index >= max_entries)) + return -1; + addrs[dma_index] = addr; addr += PAGE_SIZE; - len -= PAGE_SIZE; - index++; + dma_len -= PAGE_SIZE; + dma_index++; } } return 0; From b513b0d4563ab70a0a25e7a0ed94afaf88ac28bf Mon Sep 17 00:00:00 2001 From: Jiri Slaby Date: Thu, 19 Mar 2020 11:04:21 +0100 Subject: [PATCH 2/4] drm/virtio: fix OOB in virtio_gpu_object_create After commit f651c8b05542 ("drm/virtio: factor out the sg_table from virtio_gpu_object"), virtio_gpu_create_object allocates too small space to fit everything in. It is because it allocates struct virtio_gpu_object, but should allocate a newly added struct virtio_gpu_object_shmem which has 2 more members. So fix that by using correct type in virtio_gpu_create_object. Signed-off-by: Jiri Slaby Link: http://patchwork.freedesktop.org/patch/msgid/20200319100421.16267-1-jslaby@suse.cz Fixes: f651c8b05542 ("drm/virtio: factor out the sg_table from virtio_gpu_object") Cc: Gurchetan Singh Cc: Gerd Hoffmann Signed-off-by: Gerd Hoffmann (cherry picked from commit 0666a8d7f6a4530440e59f2d22ed4091f4d3818c) --- drivers/gpu/drm/virtio/virtgpu_object.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c b/drivers/gpu/drm/virtio/virtgpu_object.c index 2bfb13d1932e..d9039bb7c5e3 100644 --- a/drivers/gpu/drm/virtio/virtgpu_object.c +++ b/drivers/gpu/drm/virtio/virtgpu_object.c @@ -123,15 +123,17 @@ bool virtio_gpu_is_shmem(struct virtio_gpu_object *bo) struct drm_gem_object *virtio_gpu_create_object(struct drm_device *dev, size_t size) { - struct virtio_gpu_object *bo; + struct virtio_gpu_object_shmem *shmem; + struct drm_gem_shmem_object *dshmem; - bo = kzalloc(sizeof(*bo), GFP_KERNEL); - if (!bo) + shmem = kzalloc(sizeof(*shmem), GFP_KERNEL); + if (!shmem) return NULL; - bo->base.base.funcs = &virtio_gpu_shmem_funcs; - bo->base.map_cached = true; - return &bo->base.base; + dshmem = &shmem->base.base; + dshmem->base.funcs = &virtio_gpu_shmem_funcs; + dshmem->map_cached = true; + return &dshmem->base; } static int virtio_gpu_object_shmem_init(struct virtio_gpu_device *vgdev, From b2ecb89c27a4fd110187e0afeca70557215f55a1 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Thu, 2 Apr 2020 22:59:26 +0100 Subject: [PATCH 3/4] drm/legacy: Fix type for drm_local_map.offset drm_local_map.offset is not only used for resource_size_t but also dma_addr_t which may be of different sizes. Reported-by: Nathan Chancellor Fixes: 8e4ff9b56957 ("drm: Remove the dma_alloc_coherent wrapper for internal usage") Tested-by: Nathan Chancellor # build Signed-off-by: Chris Wilson Cc: Dave Airlie Cc: Nathan Chancellor Cc: Linus Torvalds Signed-off-by: Daniel Vetter Link: https://patchwork.freedesktop.org/patch/msgid/20200402215926.30714-1-chris@chris-wilson.co.uk --- include/drm/drm_legacy.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/drm/drm_legacy.h b/include/drm/drm_legacy.h index dcef3598f49e..aed382c17b26 100644 --- a/include/drm/drm_legacy.h +++ b/include/drm/drm_legacy.h @@ -136,7 +136,7 @@ struct drm_sg_mem { * Kernel side of a mapping */ struct drm_local_map { - resource_size_t offset; /**< Requested physical address (0 for SAREA)*/ + dma_addr_t offset; /**< Requested physical address (0 for SAREA)*/ unsigned long size; /**< Requested physical size (bytes) */ enum drm_map_type type; /**< Type of memory to map */ enum drm_map_flags flags; /**< Flags */ From 152cce0006abf7e17dfb7dc94896b044bda4e588 Mon Sep 17 00:00:00 2001 From: Marek Szyprowski Date: Tue, 10 Mar 2020 11:34:27 +0100 Subject: [PATCH 4/4] drm/bridge: analogix_dp: Split bind() into probe() and real bind() Analogix_dp driver acquires all its resources in the ->bind() callback, what is a bit against the component driver based approach, where the driver initialization is split into a probe(), where all resources are gathered, and a bind(), where all objects are created and a compound driver is initialized. Extract all the resource related operations to analogix_dp_probe() and analogix_dp_remove(), then call them before/after registration of the device components from the main Exynos DP and Rockchip DP drivers. Also move the plat_data initialization to the probe() to make it available for the analogix_dp_probe() function. This fixes the multiple calls to the bind() of the DRM compound driver when the DP PHY driver is not yet loaded/probed: [drm] Exynos DRM: using 14400000.fimd device for DMA mapping operations exynos-drm exynos-drm: bound 14400000.fimd (ops fimd_component_ops [exynosdrm]) exynos-drm exynos-drm: bound 14450000.mixer (ops mixer_component_ops [exynosdrm]) exynos-dp 145b0000.dp-controller: no DP phy configured exynos-drm exynos-drm: failed to bind 145b0000.dp-controller (ops exynos_dp_ops [exynosdrm]): -517 exynos-drm exynos-drm: master bind failed: -517 ... [drm] Exynos DRM: using 14400000.fimd device for DMA mapping operations exynos-drm exynos-drm: bound 14400000.fimd (ops hdmi_enable [exynosdrm]) exynos-drm exynos-drm: bound 14450000.mixer (ops hdmi_enable [exynosdrm]) exynos-drm exynos-drm: bound 145b0000.dp-controller (ops hdmi_enable [exynosdrm]) exynos-drm exynos-drm: bound 14530000.hdmi (ops hdmi_enable [exynosdrm]) [drm] Supports vblank timestamp caching Rev 2 (21.10.2013). Console: switching to colour frame buffer device 170x48 exynos-drm exynos-drm: fb0: exynosdrmfb frame buffer device [drm] Initialized exynos 1.1.0 20180330 for exynos-drm on minor 1 ... Signed-off-by: Marek Szyprowski Acked-by: Andy Yan Reviewed-by: Andrzej Hajda Signed-off-by: Andrzej Hajda Link: https://patchwork.freedesktop.org/patch/msgid/20200310103427.26048-1-m.szyprowski@samsung.com (cherry picked from commit 83a196773b8bc6702f49df1eddc848180e350340) Signed-off-by: Maxime Ripard --- .../drm/bridge/analogix/analogix_dp_core.c | 33 +++++++++++------ drivers/gpu/drm/exynos/exynos_dp.c | 29 ++++++++------- .../gpu/drm/rockchip/analogix_dp-rockchip.c | 36 ++++++++++--------- include/drm/bridge/analogix_dp.h | 5 +-- 4 files changed, 61 insertions(+), 42 deletions(-) diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c index 9ded2cef57dd..76736fb8ed94 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c @@ -1652,8 +1652,7 @@ static ssize_t analogix_dpaux_transfer(struct drm_dp_aux *aux, } struct analogix_dp_device * -analogix_dp_bind(struct device *dev, struct drm_device *drm_dev, - struct analogix_dp_plat_data *plat_data) +analogix_dp_probe(struct device *dev, struct analogix_dp_plat_data *plat_data) { struct platform_device *pdev = to_platform_device(dev); struct analogix_dp_device *dp; @@ -1756,22 +1755,30 @@ analogix_dp_bind(struct device *dev, struct drm_device *drm_dev, irq_flags, "analogix-dp", dp); if (ret) { dev_err(&pdev->dev, "failed to request irq\n"); - goto err_disable_pm_runtime; + return ERR_PTR(ret); } disable_irq(dp->irq); + return dp; +} +EXPORT_SYMBOL_GPL(analogix_dp_probe); + +int analogix_dp_bind(struct analogix_dp_device *dp, struct drm_device *drm_dev) +{ + int ret; + dp->drm_dev = drm_dev; dp->encoder = dp->plat_data->encoder; dp->aux.name = "DP-AUX"; dp->aux.transfer = analogix_dpaux_transfer; - dp->aux.dev = &pdev->dev; + dp->aux.dev = dp->dev; ret = drm_dp_aux_register(&dp->aux); if (ret) - return ERR_PTR(ret); + return ret; - pm_runtime_enable(dev); + pm_runtime_enable(dp->dev); ret = analogix_dp_create_bridge(drm_dev, dp); if (ret) { @@ -1779,13 +1786,12 @@ analogix_dp_bind(struct device *dev, struct drm_device *drm_dev, goto err_disable_pm_runtime; } - return dp; + return 0; err_disable_pm_runtime: + pm_runtime_disable(dp->dev); - pm_runtime_disable(dev); - - return ERR_PTR(ret); + return ret; } EXPORT_SYMBOL_GPL(analogix_dp_bind); @@ -1802,10 +1808,15 @@ void analogix_dp_unbind(struct analogix_dp_device *dp) drm_dp_aux_unregister(&dp->aux); pm_runtime_disable(dp->dev); - clk_disable_unprepare(dp->clock); } EXPORT_SYMBOL_GPL(analogix_dp_unbind); +void analogix_dp_remove(struct analogix_dp_device *dp) +{ + clk_disable_unprepare(dp->clock); +} +EXPORT_SYMBOL_GPL(analogix_dp_remove); + #ifdef CONFIG_PM int analogix_dp_suspend(struct analogix_dp_device *dp) { diff --git a/drivers/gpu/drm/exynos/exynos_dp.c b/drivers/gpu/drm/exynos/exynos_dp.c index d23d3502ca91..5ee090691390 100644 --- a/drivers/gpu/drm/exynos/exynos_dp.c +++ b/drivers/gpu/drm/exynos/exynos_dp.c @@ -159,15 +159,8 @@ static int exynos_dp_bind(struct device *dev, struct device *master, void *data) struct drm_device *drm_dev = data; int ret; - dp->dev = dev; dp->drm_dev = drm_dev; - dp->plat_data.dev_type = EXYNOS_DP; - dp->plat_data.power_on_start = exynos_dp_poweron; - dp->plat_data.power_off = exynos_dp_poweroff; - dp->plat_data.attach = exynos_dp_bridge_attach; - dp->plat_data.get_modes = exynos_dp_get_modes; - if (!dp->plat_data.panel && !dp->ptn_bridge) { ret = exynos_dp_dt_parse_panel(dp); if (ret) @@ -185,13 +178,11 @@ static int exynos_dp_bind(struct device *dev, struct device *master, void *data) dp->plat_data.encoder = encoder; - dp->adp = analogix_dp_bind(dev, dp->drm_dev, &dp->plat_data); - if (IS_ERR(dp->adp)) { + ret = analogix_dp_bind(dp->adp, dp->drm_dev); + if (ret) dp->encoder.funcs->destroy(&dp->encoder); - return PTR_ERR(dp->adp); - } - return 0; + return ret; } static void exynos_dp_unbind(struct device *dev, struct device *master, @@ -222,6 +213,7 @@ static int exynos_dp_probe(struct platform_device *pdev) if (!dp) return -ENOMEM; + dp->dev = dev; /* * We just use the drvdata until driver run into component * add function, and then we would set drvdata to null, so @@ -247,16 +239,29 @@ static int exynos_dp_probe(struct platform_device *pdev) /* The remote port can be either a panel or a bridge */ dp->plat_data.panel = panel; + dp->plat_data.dev_type = EXYNOS_DP; + dp->plat_data.power_on_start = exynos_dp_poweron; + dp->plat_data.power_off = exynos_dp_poweroff; + dp->plat_data.attach = exynos_dp_bridge_attach; + dp->plat_data.get_modes = exynos_dp_get_modes; dp->plat_data.skip_connector = !!bridge; + dp->ptn_bridge = bridge; out: + dp->adp = analogix_dp_probe(dev, &dp->plat_data); + if (IS_ERR(dp->adp)) + return PTR_ERR(dp->adp); + return component_add(&pdev->dev, &exynos_dp_ops); } static int exynos_dp_remove(struct platform_device *pdev) { + struct exynos_dp_device *dp = platform_get_drvdata(pdev); + component_del(&pdev->dev, &exynos_dp_ops); + analogix_dp_remove(dp->adp); return 0; } diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c index f38f5e113c6b..ce98c08aa8b4 100644 --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c @@ -325,15 +325,9 @@ static int rockchip_dp_bind(struct device *dev, struct device *master, void *data) { struct rockchip_dp_device *dp = dev_get_drvdata(dev); - const struct rockchip_dp_chip_data *dp_data; struct drm_device *drm_dev = data; int ret; - dp_data = of_device_get_match_data(dev); - if (!dp_data) - return -ENODEV; - - dp->data = dp_data; dp->drm_dev = drm_dev; ret = rockchip_dp_drm_create_encoder(dp); @@ -344,16 +338,9 @@ static int rockchip_dp_bind(struct device *dev, struct device *master, dp->plat_data.encoder = &dp->encoder; - dp->plat_data.dev_type = dp->data->chip_type; - dp->plat_data.power_on_start = rockchip_dp_poweron_start; - dp->plat_data.power_off = rockchip_dp_powerdown; - dp->plat_data.get_modes = rockchip_dp_get_modes; - - dp->adp = analogix_dp_bind(dev, dp->drm_dev, &dp->plat_data); - if (IS_ERR(dp->adp)) { - ret = PTR_ERR(dp->adp); + ret = analogix_dp_bind(dp->adp, drm_dev); + if (ret) goto err_cleanup_encoder; - } return 0; err_cleanup_encoder: @@ -368,8 +355,6 @@ static void rockchip_dp_unbind(struct device *dev, struct device *master, analogix_dp_unbind(dp->adp); dp->encoder.funcs->destroy(&dp->encoder); - - dp->adp = ERR_PTR(-ENODEV); } static const struct component_ops rockchip_dp_component_ops = { @@ -380,10 +365,15 @@ static const struct component_ops rockchip_dp_component_ops = { static int rockchip_dp_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev; + const struct rockchip_dp_chip_data *dp_data; struct drm_panel *panel = NULL; struct rockchip_dp_device *dp; int ret; + dp_data = of_device_get_match_data(dev); + if (!dp_data) + return -ENODEV; + ret = drm_of_find_panel_or_bridge(dev->of_node, 1, 0, &panel, NULL); if (ret < 0) return ret; @@ -394,7 +384,12 @@ static int rockchip_dp_probe(struct platform_device *pdev) dp->dev = dev; dp->adp = ERR_PTR(-ENODEV); + dp->data = dp_data; dp->plat_data.panel = panel; + dp->plat_data.dev_type = dp->data->chip_type; + dp->plat_data.power_on_start = rockchip_dp_poweron_start; + dp->plat_data.power_off = rockchip_dp_powerdown; + dp->plat_data.get_modes = rockchip_dp_get_modes; ret = rockchip_dp_of_probe(dp); if (ret < 0) @@ -402,12 +397,19 @@ static int rockchip_dp_probe(struct platform_device *pdev) platform_set_drvdata(pdev, dp); + dp->adp = analogix_dp_probe(dev, &dp->plat_data); + if (IS_ERR(dp->adp)) + return PTR_ERR(dp->adp); + return component_add(dev, &rockchip_dp_component_ops); } static int rockchip_dp_remove(struct platform_device *pdev) { + struct rockchip_dp_device *dp = platform_get_drvdata(pdev); + component_del(&pdev->dev, &rockchip_dp_component_ops); + analogix_dp_remove(dp->adp); return 0; } diff --git a/include/drm/bridge/analogix_dp.h b/include/drm/bridge/analogix_dp.h index 7aa2f93da49c..b0dcc07334a1 100644 --- a/include/drm/bridge/analogix_dp.h +++ b/include/drm/bridge/analogix_dp.h @@ -42,9 +42,10 @@ int analogix_dp_resume(struct analogix_dp_device *dp); int analogix_dp_suspend(struct analogix_dp_device *dp); struct analogix_dp_device * -analogix_dp_bind(struct device *dev, struct drm_device *drm_dev, - struct analogix_dp_plat_data *plat_data); +analogix_dp_probe(struct device *dev, struct analogix_dp_plat_data *plat_data); +int analogix_dp_bind(struct analogix_dp_device *dp, struct drm_device *drm_dev); void analogix_dp_unbind(struct analogix_dp_device *dp); +void analogix_dp_remove(struct analogix_dp_device *dp); int analogix_dp_start_crc(struct drm_connector *connector); int analogix_dp_stop_crc(struct drm_connector *connector);