Re: [PATCH v2 0/3] ast, mgag200: Map console BO while it's being displayed
Hi, > - imo we should fix this by using the io_mapping stuff, that avoids > the overhead of repeated pat checks for map/unmap. Another idea: IIRC ttm has a move_notify callback. So we could simply keep mappings active even when the refcount goes down to zero. Then do the actual unmap either in the move_notify or in the destroy callback. cheers, Gerd ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/8] drm/ttm: turn ttm_bo_device.vma_manager into a pointer
Rename the embedded struct vma_offset_manager, new name is _vma_manager. ttm_bo_device.vma_manager changed to a pointer. The ttm_bo_device_init() function gets an additional vma_manager argument which allows to initialize ttm with a different vma manager. When passing NULL the embedded _vma_manager is used. All callers are updated to pass NULL, so the behavior doesn't change. Signed-off-by: Gerd Hoffmann --- include/drm/ttm/ttm_bo_driver.h | 8 ++-- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 1 + drivers/gpu/drm/drm_vram_mm_helper.c| 1 + drivers/gpu/drm/nouveau/nouveau_ttm.c | 1 + drivers/gpu/drm/qxl/qxl_ttm.c | 1 + drivers/gpu/drm/radeon/radeon_ttm.c | 1 + drivers/gpu/drm/ttm/ttm_bo.c| 13 + drivers/gpu/drm/ttm/ttm_bo_vm.c | 6 +++--- drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 1 + 9 files changed, 24 insertions(+), 9 deletions(-) diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h index e88e00c6cbf2..e365434f92b3 100644 --- a/include/drm/ttm/ttm_bo_driver.h +++ b/include/drm/ttm/ttm_bo_driver.h @@ -441,7 +441,8 @@ extern struct ttm_bo_global { * * @driver: Pointer to a struct ttm_bo_driver struct setup by the driver. * @man: An array of mem_type_managers. - * @vma_manager: Address space manager + * @vma_manager: Address space manager (pointer) + * @_vma_manager: Address space manager (enbedded) * lru_lock: Spinlock that protects the buffer+device lru lists and * ddestroy lists. * @dev_mapping: A pointer to the struct address_space representing the @@ -464,7 +465,8 @@ struct ttm_bo_device { /* * Protected by internal locks. */ - struct drm_vma_offset_manager vma_manager; + struct drm_vma_offset_manager *vma_manager; + struct drm_vma_offset_manager _vma_manager; /* * Protected by the global:lru lock. @@ -585,6 +587,7 @@ int ttm_bo_device_release(struct ttm_bo_device *bdev); * @glob: A pointer to an initialized struct ttm_bo_global. * @driver: A pointer to a struct ttm_bo_driver set up by the caller. * @mapping: The address space to use for this bo. + * @vma_manager: A pointer to a vma manager or NULL. * @file_page_offset: Offset into the device address space that is available * for buffer data. This ensures compatibility with other users of the * address space. @@ -596,6 +599,7 @@ int ttm_bo_device_release(struct ttm_bo_device *bdev); int ttm_bo_device_init(struct ttm_bo_device *bdev, struct ttm_bo_driver *driver, struct address_space *mapping, + struct drm_vma_offset_manager *vma_manager, bool need_dma32); /** diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c index fb09314bcfd4..34ee5d725faf 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c @@ -1728,6 +1728,7 @@ int amdgpu_ttm_init(struct amdgpu_device *adev) r = ttm_bo_device_init(&adev->mman.bdev, &amdgpu_bo_driver, adev->ddev->anon_inode->i_mapping, + NULL, adev->need_dma32); if (r) { DRM_ERROR("failed initializing buffer object driver(%d).\n", r); diff --git a/drivers/gpu/drm/drm_vram_mm_helper.c b/drivers/gpu/drm/drm_vram_mm_helper.c index c911781d6728..56fd1519eb35 100644 --- a/drivers/gpu/drm/drm_vram_mm_helper.c +++ b/drivers/gpu/drm/drm_vram_mm_helper.c @@ -172,6 +172,7 @@ int drm_vram_mm_init(struct drm_vram_mm *vmm, struct drm_device *dev, ret = ttm_bo_device_init(&vmm->bdev, &bo_driver, dev->anon_inode->i_mapping, +NULL, true); if (ret) return ret; diff --git a/drivers/gpu/drm/nouveau/nouveau_ttm.c b/drivers/gpu/drm/nouveau/nouveau_ttm.c index f0daf958e03a..e67eb10843d1 100644 --- a/drivers/gpu/drm/nouveau/nouveau_ttm.c +++ b/drivers/gpu/drm/nouveau/nouveau_ttm.c @@ -236,6 +236,7 @@ nouveau_ttm_init(struct nouveau_drm *drm) ret = ttm_bo_device_init(&drm->ttm.bdev, &nouveau_bo_driver, dev->anon_inode->i_mapping, +NULL, drm->client.mmu.dmabits <= 32 ? true : false); if (ret) { NV_ERROR(drm, "error initialising bo driver, %d\n", ret); diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c index 9b24514c75aa..69da0eea6e4c 100644 --- a/drivers/gpu/drm/qxl/qxl_ttm.c +++ b/drivers/gpu/drm/qxl/qxl_ttm.c @@ -325,6 +325,7 @@ int qxl_ttm_init(struct qxl_device *qdev) r = ttm_bo_device_init(&qdev->mman.bdev, &qxl_bo_driver, qdev->ddev.anon_inode
[PATCH 7/8] drm/vmwgfx: switch to own vma manager
Add struct drm_vma_offset_manager to vma_private, initialize it and pass it to ttm_bo_device_init(). With this in place the last user of ttm's embedded vma offset manager is gone and we can remove it (in a separate patch). Signed-off-by: Gerd Hoffmann --- drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 1 + drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 6 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h index dbb04dbcf478..adb0436528c7 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h @@ -420,6 +420,7 @@ struct vmw_private { struct vmw_fifo_state fifo; struct drm_device *dev; + struct drm_vma_offset_manager vma_manager; unsigned long vmw_chipset; unsigned int io_start; uint32_t vram_start; diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c index 20bc91214e75..882facd055de 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c @@ -827,10 +827,13 @@ static int vmw_driver_load(struct drm_device *dev, unsigned long chipset) goto out_no_fman; } + drm_vma_offset_manager_init(&dev_priv->vma_manager, + DRM_FILE_PAGE_OFFSET_START, + DRM_FILE_PAGE_OFFSET_SIZE); ret = ttm_bo_device_init(&dev_priv->bdev, &vmw_bo_driver, dev->anon_inode->i_mapping, -NULL, +&dev_priv->vma_manager, false); if (unlikely(ret != 0)) { DRM_ERROR("Failed initializing TTM buffer object driver.\n"); @@ -987,6 +990,7 @@ static void vmw_driver_unload(struct drm_device *dev) if (dev_priv->has_mob) (void) ttm_bo_clean_mm(&dev_priv->bdev, VMW_PL_MOB); (void) ttm_bo_device_release(&dev_priv->bdev); + drm_vma_offset_manager_destroy(&dev_priv->vma_manager); vmw_release_device_late(dev_priv); vmw_fence_manager_takedown(dev_priv->fman); if (dev_priv->capabilities & SVGA_CAP_IRQMASK) -- 2.18.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 6/8] drm/qxl: switch to gem vma offset manager
Pass gem vma_offset_manager to ttm_bo_device_init(), so ttm uses it instead of its own embedded struct. This makes some gem functions (specifically drm_gem_object_lookup) work on ttm objects. Signed-off-by: Gerd Hoffmann --- drivers/gpu/drm/qxl/qxl_ttm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c index 69da0eea6e4c..cbc6c2ba8630 100644 --- a/drivers/gpu/drm/qxl/qxl_ttm.c +++ b/drivers/gpu/drm/qxl/qxl_ttm.c @@ -325,7 +325,7 @@ int qxl_ttm_init(struct qxl_device *qdev) r = ttm_bo_device_init(&qdev->mman.bdev, &qxl_bo_driver, qdev->ddev.anon_inode->i_mapping, - NULL, + qdev->ddev.vma_offset_manager, false); if (r) { DRM_ERROR("failed initializing buffer object driver(%d).\n", r); -- 2.18.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 5/8] drm/amdgpu: switch to gem vma offset manager
Pass gem vma_offset_manager to ttm_bo_device_init(), so ttm uses it instead of its own embedded struct. This makes some gem functions (specifically drm_gem_object_lookup) work on ttm objects. Signed-off-by: Gerd Hoffmann --- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c index 34ee5d725faf..513dd8456945 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c @@ -1728,7 +1728,7 @@ int amdgpu_ttm_init(struct amdgpu_device *adev) r = ttm_bo_device_init(&adev->mman.bdev, &amdgpu_bo_driver, adev->ddev->anon_inode->i_mapping, - NULL, + adev->ddev->vma_offset_manager, adev->need_dma32); if (r) { DRM_ERROR("failed initializing buffer object driver(%d).\n", r); -- 2.18.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 0/8] drm/ttm: remove embedded vma_manager from ttm_bo_device
Gerd Hoffmann (8): drm/ttm: turn ttm_bo_device.vma_manager into a pointer drm/nouveau: switch to gem vma offset manager drm/vram: switch to gem vma offset manager drm/radeon: switch to gem vma offset manager drm/amdgpu: switch to gem vma offset manager drm/qxl: switch to gem vma offset manager drm/vmwgfx: switch to own vma manager drm/ttm: remove embedded vma_offset_manager drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 1 + include/drm/ttm/ttm_bo_driver.h | 6 -- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 1 + drivers/gpu/drm/drm_vram_mm_helper.c| 1 + drivers/gpu/drm/nouveau/nouveau_ttm.c | 1 + drivers/gpu/drm/qxl/qxl_ttm.c | 1 + drivers/gpu/drm/radeon/radeon_ttm.c | 1 + drivers/gpu/drm/ttm/ttm_bo.c| 14 +++--- drivers/gpu/drm/ttm/ttm_bo_vm.c | 6 +++--- drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 5 + 10 files changed, 25 insertions(+), 12 deletions(-) -- 2.18.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 8/8] drm/ttm: remove embedded vma_offset_manager
No users left. Drivers either setup vma_offset_manager themself (vmwgfx) or pass the gem vma_offset_manager to ttm_bo_device_init (all other drivers). Signed-off-by: Gerd Hoffmann --- include/drm/ttm/ttm_bo_driver.h | 4 +--- drivers/gpu/drm/ttm/ttm_bo.c| 9 ++--- 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h index e365434f92b3..4e307f65f497 100644 --- a/include/drm/ttm/ttm_bo_driver.h +++ b/include/drm/ttm/ttm_bo_driver.h @@ -442,7 +442,6 @@ extern struct ttm_bo_global { * @driver: Pointer to a struct ttm_bo_driver struct setup by the driver. * @man: An array of mem_type_managers. * @vma_manager: Address space manager (pointer) - * @_vma_manager: Address space manager (enbedded) * lru_lock: Spinlock that protects the buffer+device lru lists and * ddestroy lists. * @dev_mapping: A pointer to the struct address_space representing the @@ -466,7 +465,6 @@ struct ttm_bo_device { * Protected by internal locks. */ struct drm_vma_offset_manager *vma_manager; - struct drm_vma_offset_manager _vma_manager; /* * Protected by the global:lru lock. @@ -587,7 +585,7 @@ int ttm_bo_device_release(struct ttm_bo_device *bdev); * @glob: A pointer to an initialized struct ttm_bo_global. * @driver: A pointer to a struct ttm_bo_driver set up by the caller. * @mapping: The address space to use for this bo. - * @vma_manager: A pointer to a vma manager or NULL. + * @vma_manager: A pointer to a vma manager. * @file_page_offset: Offset into the device address space that is available * for buffer data. This ensures compatibility with other users of the * address space. diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 8dc26babc5cb..881cf26d698e 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -1704,8 +1704,6 @@ int ttm_bo_device_release(struct ttm_bo_device *bdev) pr_debug("Swap list %d was clean\n", i); spin_unlock(&glob->lru_lock); - drm_vma_offset_manager_destroy(&bdev->_vma_manager); - if (!ret) ttm_bo_global_release(); @@ -1722,8 +1720,8 @@ int ttm_bo_device_init(struct ttm_bo_device *bdev, struct ttm_bo_global *glob = &ttm_bo_glob; int ret; - if (!vma_manager) - vma_manager = &bdev->_vma_manager; + if (WARN_ON(vma_manager == NULL)) + return -EINVAL; ret = ttm_bo_global_init(); if (ret) @@ -1742,9 +1740,6 @@ int ttm_bo_device_init(struct ttm_bo_device *bdev, goto out_no_sys; bdev->vma_manager = vma_manager; - drm_vma_offset_manager_init(&bdev->_vma_manager, - DRM_FILE_PAGE_OFFSET_START, - DRM_FILE_PAGE_OFFSET_SIZE); INIT_DELAYED_WORK(&bdev->wq, ttm_bo_delayed_workqueue); INIT_LIST_HEAD(&bdev->ddestroy); bdev->dev_mapping = mapping; -- 2.18.1
[PATCH 3/8] drm/vram: switch to gem vma offset manager
Pass gem vma_offset_manager to ttm_bo_device_init(), so ttm uses it instead of its own embedded struct. This makes some gem functions (specifically drm_gem_object_lookup) work on ttm objects. Signed-off-by: Gerd Hoffmann --- drivers/gpu/drm/drm_vram_mm_helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_vram_mm_helper.c b/drivers/gpu/drm/drm_vram_mm_helper.c index 56fd1519eb35..3b2552bec4e6 100644 --- a/drivers/gpu/drm/drm_vram_mm_helper.c +++ b/drivers/gpu/drm/drm_vram_mm_helper.c @@ -172,7 +172,7 @@ int drm_vram_mm_init(struct drm_vram_mm *vmm, struct drm_device *dev, ret = ttm_bo_device_init(&vmm->bdev, &bo_driver, dev->anon_inode->i_mapping, -NULL, +dev->vma_offset_manager, true); if (ret) return ret; -- 2.18.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 4/8] drm/radeon: switch to gem vma offset manager
Pass gem vma_offset_manager to ttm_bo_device_init(), so ttm uses it instead of its own embedded struct. This makes some gem functions (specifically drm_gem_object_lookup) work on ttm objects. Signed-off-by: Gerd Hoffmann --- drivers/gpu/drm/radeon/radeon_ttm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c index e2c12e59704d..a46bb3067863 100644 --- a/drivers/gpu/drm/radeon/radeon_ttm.c +++ b/drivers/gpu/drm/radeon/radeon_ttm.c @@ -794,7 +794,7 @@ int radeon_ttm_init(struct radeon_device *rdev) r = ttm_bo_device_init(&rdev->mman.bdev, &radeon_bo_driver, rdev->ddev->anon_inode->i_mapping, - NULL, + rdev->ddev->vma_offset_manager, rdev->need_dma32); if (r) { DRM_ERROR("failed initializing buffer object driver(%d).\n", r); -- 2.18.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/8] drm/nouveau: switch to gem vma offset manager
Pass gem vma_offset_manager to ttm_bo_device_init(), so ttm uses it instead of its own embedded struct. This makes some gem functions (specifically drm_gem_object_lookup) work on ttm objects. Signed-off-by: Gerd Hoffmann --- drivers/gpu/drm/nouveau/nouveau_ttm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/nouveau/nouveau_ttm.c b/drivers/gpu/drm/nouveau/nouveau_ttm.c index e67eb10843d1..77a0c6ad3cef 100644 --- a/drivers/gpu/drm/nouveau/nouveau_ttm.c +++ b/drivers/gpu/drm/nouveau/nouveau_ttm.c @@ -236,7 +236,7 @@ nouveau_ttm_init(struct nouveau_drm *drm) ret = ttm_bo_device_init(&drm->ttm.bdev, &nouveau_bo_driver, dev->anon_inode->i_mapping, -NULL, + dev->vma_offset_manager, drm->client.mmu.dmabits <= 32 ? true : false); if (ret) { NV_ERROR(drm, "error initialising bo driver, %d\n", ret); -- 2.18.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/amdgpu: Delete an unnecessary check before two function calls
From: Markus Elfring Date: Wed, 4 Sep 2019 12:30:23 +0200 The functions “debugfs_remove” and “kfree” test whether their argument is NULL and then return immediately. Thus the tests around the shown calls are not needed. This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring --- drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 8 ++-- 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c index 5652cc72ed3a..d321c72d63d1 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c @@ -1076,10 +1076,7 @@ static int amdgpu_debugfs_ib_preempt(void *data, u64 val) kthread_unpark(ring->sched.thread); ttm_bo_unlock_delayed_workqueue(&adev->mman.bdev, resched); - - if (fences) - kfree(fences); - + kfree(fences); return 0; } @@ -1103,8 +1100,7 @@ int amdgpu_debugfs_init(struct amdgpu_device *adev) void amdgpu_debugfs_preempt_cleanup(struct amdgpu_device *adev) { - if (adev->debugfs_preempt) - debugfs_remove(adev->debugfs_preempt); + debugfs_remove(adev->debugfs_preempt); } #else -- 2.23.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH -next] fbdev: s3c-fb: use devm_platform_ioremap_resource() to simplify code
Use devm_platform_ioremap_resource() to simplify the code a bit. This is detected by coccinelle. Reported-by: Hulk Robot Signed-off-by: YueHaibing --- drivers/video/fbdev/s3c-fb.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/video/fbdev/s3c-fb.c b/drivers/video/fbdev/s3c-fb.c index ba04d7a..43ac8d7 100644 --- a/drivers/video/fbdev/s3c-fb.c +++ b/drivers/video/fbdev/s3c-fb.c @@ -1411,8 +1411,7 @@ static int s3c_fb_probe(struct platform_device *pdev) pm_runtime_enable(sfb->dev); - res = platform_get_resource(pdev, IORESOURCE_MEM, 0); - sfb->regs = devm_ioremap_resource(dev, res); + sfb->regs = devm_platform_ioremap_resource(pdev, 0); if (IS_ERR(sfb->regs)) { ret = PTR_ERR(sfb->regs); goto err_lcd_clk; -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v18 15/15] selftests, arm64: add a selftest for passing tagged pointers to kernel
On Wed, Sep 4, 2019 at 6:22 PM Cristian Marussi wrote: > > Hi Andrey ! > > On 04/09/2019 15:52, Andrey Konovalov wrote: > > On Fri, Aug 23, 2019 at 7:49 PM Cristian Marussi > > wrote: > >> > >> > >> Hi > >> > >> On 23/08/2019 18:16, Andrey Konovalov wrote: > >>> On Fri, Aug 23, 2019 at 3:56 PM Cristian Marussi > >>> wrote: > > Hi Andrey > > On 24/06/2019 15:33, Andrey Konovalov wrote: > > This patch is a part of a series that extends kernel ABI to allow to > > pass > > tagged user pointers (with the top byte set to something else other than > > 0x00) as syscall arguments. > > > > This patch adds a simple test, that calls the uname syscall with a > > tagged user pointer as an argument. Without the kernel accepting tagged > > user pointers the test fails with EFAULT. > > > > Signed-off-by: Andrey Konovalov > > --- > > tools/testing/selftests/arm64/.gitignore | 1 + > > tools/testing/selftests/arm64/Makefile| 11 +++ > > .../testing/selftests/arm64/run_tags_test.sh | 12 > > tools/testing/selftests/arm64/tags_test.c | 29 +++ > > 4 files changed, 53 insertions(+) > > create mode 100644 tools/testing/selftests/arm64/.gitignore > > create mode 100644 tools/testing/selftests/arm64/Makefile > > create mode 100755 tools/testing/selftests/arm64/run_tags_test.sh > > create mode 100644 tools/testing/selftests/arm64/tags_test.c > > After building a fresh Kernel from arm64/for-next-core from scratch at: > > commit 239ab658bea3b387424501e7c416640d6752dc0c > Merge: 6bfa3134bd3a 42d038c4fb00 1243cb6a676f d55c5f28afaf d06fa5a118f1 > 34b5560db40d > Author: Will Deacon > Date: Thu Aug 22 18:23:53 2019 +0100 > > Merge branches 'for-next/error-injection', 'for-next/tbi', > 'for-next/psci-cpuidle', 'for-next/cpu-topology' and > 'for-next/52-bit-kva' into for-next/core > > > KSFT arm64 tests build is broken for me, both setting or not > KBUILD_OUTPUT= > > 13:30 $ make TARGETS=arm64 kselftest-clean > make[1]: Entering directory '/home/crimar01/ARM/dev/src/pdsw/out_linux' > rm -f -r > /home/crimar01/ARM/dev/src/pdsw/out_linux//kselftest/arm64/tags_test > make[1]: Leaving directory '/home/crimar01/ARM/dev/src/pdsw/out_linux' > > ✔ ~/ARM/dev/src/pdsw/linux [arm64_for_next_core|…8⚑ 23] > > 13:30 $ make TARGETS=arm64 kselftest > make[1]: Entering directory '/home/crimar01/ARM/dev/src/pdsw/out_linux' > arch/arm64/Makefile:56: CROSS_COMPILE_COMPAT not defined or empty, the > compat vDSO will not be built > make --no-builtin-rules INSTALL_HDR_PATH=$BUILD/usr \ > ARCH=arm64 -C ../../.. headers_install > HOSTCC scripts/basic/fixdep > HOSTCC scripts/unifdef > ... > ... > HDRINST usr/include/asm/msgbuf.h > HDRINST usr/include/asm/shmbuf.h > INSTALL > /home/crimar01/ARM/dev/src/pdsw/out_linux//kselftest/usr/include > /opt/toolchains/gcc-arm-8.3-2019.03-x86_64-aarch64-linux-gnu/bin/aarch64-linux-gnu-gcc > tags_test.c -o > /home/crimar01/ARM/dev/src/pdsw/out_linux//kselftest/arm64/tags_test > tags_test.c: In function ‘main’: > tags_test.c:21:12: error: ‘PR_SET_TAGGED_ADDR_CTRL’ undeclared (first > use in this function); did you mean ‘PR_GET_TID_ADDRESS’? > if (prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE, 0, 0, 0) == > 0) > ^~~ > PR_GET_TID_ADDRESS > tags_test.c:21:12: note: each undeclared identifier is reported only > once for each function it appears in > tags_test.c:21:37: error: ‘PR_TAGGED_ADDR_ENABLE’ undeclared (first use > in this function); did you mean ‘PR_GET_DUMPABLE’? > if (prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE, 0, 0, 0) == > 0) > ^ > PR_GET_DUMPABLE > ../lib.mk:138: recipe for target > '/home/crimar01/ARM/dev/src/pdsw/out_linux//kselftest/arm64/tags_test' > failed > make[3]: *** > [/home/crimar01/ARM/dev/src/pdsw/out_linux//kselftest/arm64/tags_test] > Error 1 > Makefile:136: recipe for target 'all' failed > make[2]: *** [all] Error 2 > /home/crimar01/ARM/dev/src/pdsw/linux/Makefile:1237: recipe for target > 'kselftest' failed > make[1]: *** [kselftest] Error 2 > make[1]: Leaving directory '/home/crimar01/ARM/dev/src/pdsw/out_linux' > Makefile:179: recipe for target 'sub-make' failed > make: *** [sub-make] Error 2 > > Despite seeing KSFT installing Kernel Headers, they cannot be found. > > Fixing this patch like this make it work for me: > > diff --git a/tools/testing/selftests/arm64/Ma
[PATCH] drm/ttm: let ttm_bo_wait timeout be configurable
When running dEQP against virgl driver, it turns out the default 15 seconds timeout for ttm_bo_wait is not big enough for GLES31.functional.ssbo.layout.random.nested_structs_arrays_instance_arrays.22 Change it to a configurable value so we can tune it before virgl performance gets improved. Signed-off-by: Lepton Wu --- drivers/gpu/drm/Kconfig | 2 ++ drivers/gpu/drm/ttm/Kconfig | 7 +++ drivers/gpu/drm/ttm/ttm_bo.c | 2 +- 3 files changed, 10 insertions(+), 1 deletion(-) create mode 100644 drivers/gpu/drm/ttm/Kconfig diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index bd943a71756c..432054012fa1 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -160,6 +160,8 @@ config DRM_TTM GPU memory types. Will be enabled automatically if a device driver uses it. +source "drivers/gpu/drm/ttm/Kconfig" + config DRM_GEM_CMA_HELPER bool depends on DRM diff --git a/drivers/gpu/drm/ttm/Kconfig b/drivers/gpu/drm/ttm/Kconfig new file mode 100644 index ..c7953271c59b --- /dev/null +++ b/drivers/gpu/drm/ttm/Kconfig @@ -0,0 +1,7 @@ +config DRM_TTM_BO_WAIT_TIMEOUT + int "Default timeout for ttm bo wait (in seconds)" + depends on DRM_TTM + default 15 + help + This option controls the default timeout (in seconds) used in + ttm_bo_wait diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 3f56647cdb35..fb6991811ede 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -1709,7 +1709,7 @@ EXPORT_SYMBOL(ttm_bo_unmap_virtual); int ttm_bo_wait(struct ttm_buffer_object *bo, bool interruptible, bool no_wait) { - long timeout = 15 * HZ; + long timeout = CONFIG_DRM_TTM_BO_WAIT_TIMEOUT * HZ; if (no_wait) { if (reservation_object_test_signaled_rcu(bo->resv, true)) -- 2.23.0.187.g17f5b7556c-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [BACKPORT 4.14.y 1/8] drm/i915/fbdev: Actually configure untiled displays
On Thu, 5 Sep 2019 at 01:25, Greg KH wrote: > > On Tue, Sep 03, 2019 at 02:55:26PM +0800, Baolin Wang wrote: > > From: Chris Wilson > > > > If we skipped all the connectors that were not part of a tile, we would > > leave conn_seq=0 and conn_configured=0, convincing ourselves that we > > had stagnated in our configuration attempts. Avoid this situation by > > starting conn_seq=ALL_CONNECTORS, and repeating until we find no more > > connectors to configure. > > > > Fixes: 754a76591b12 ("drm/i915/fbdev: Stop repeating tile configuration on > > stagnation") > > Reported-by: Maarten Lankhorst > > Signed-off-by: Chris Wilson > > Cc: Maarten Lankhorst > > Reviewed-by: Maarten Lankhorst > > Link: > > https://patchwork.freedesktop.org/patch/msgid/20190215123019.32283-1-ch...@chris-wilson.co.uk > > Cc: # v3.19+ > > Signed-off-by: Baolin Wang > > --- > > drivers/gpu/drm/i915/intel_fbdev.c | 12 +++- > > 1 file changed, 7 insertions(+), 5 deletions(-) > > What is the git commit id of this patch in Linus's tree? The commit id is: d9b308b1f8a1acc0c3279f443d4fe0f9f663252e > > Can you please add that as the first line of the changelog like is done > with all other stable patches? That way I can verify that what you > posted here is the correct one. > > Please fix the up for all of these and resend. Sure. Thanks. -- Baolin Wang Best Regards ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: drm/amdgpu: Delete an unnecessary check before two function calls
> The functions “debugfs_remove” and “kfree” test whether their argument > is NULL and then return immediately. > Thus the tests around the shown calls are not needed. > > This issue was detected by using the Coccinelle software. I suggest to take another look at a similar patch. drm/amdgpu: remove the redundant null check https://lkml.org/lkml/2019/9/3/59 https://lore.kernel.org/patchwork/patch/1123118/ https://lore.kernel.org/r/1567491305-18320-1-git-send-email-zhongji...@huawei.com/ Regards, Markus ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 3/3] drm/edid: no CEA v3 extension is not an error
It is fine for displays without audio functionality to not implement CEA v3 extension in their EDID. Do not return an error in that case, instead return 0 as if there was a CEA v3 extension with no audio or speaker block. This fixes half of bug fdo#107825: https://bugs.freedesktop.org/show_bug.cgi?id=107825 Signed-off-by: Jean Delvare Reviewed-by: Ville Syrjälä Cc: Maarten Lankhorst Cc: Maxime Ripard Cc: Sean Paul Cc: David Airlie Cc: Daniel Vetter --- Changes since v1: * Treat CEA extension version < 3 as non-error too (suggested by Ville Syrjälä) drivers/gpu/drm/drm_edid.c |8 1 file changed, 4 insertions(+), 4 deletions(-) --- linux-5.2.orig/drivers/gpu/drm/drm_edid.c 2019-09-02 10:21:01.495525663 +0200 +++ linux-5.2/drivers/gpu/drm/drm_edid.c2019-09-04 10:33:51.080273331 +0200 @@ -4130,12 +4130,12 @@ int drm_edid_to_sad(struct edid *edid, s cea = drm_find_cea_extension(edid); if (!cea) { DRM_DEBUG_KMS("SAD: no CEA Extension found\n"); - return -ENOENT; + return 0; } if (cea_revision(cea) < 3) { DRM_DEBUG_KMS("SAD: wrong CEA revision\n"); - return -ENOTSUPP; + return 0; } if (cea_db_offsets(cea, &start, &end)) { @@ -4191,12 +4191,12 @@ int drm_edid_to_speaker_allocation(struc cea = drm_find_cea_extension(edid); if (!cea) { DRM_DEBUG_KMS("SAD: no CEA Extension found\n"); - return -ENOENT; + return 0; } if (cea_revision(cea) < 3) { DRM_DEBUG_KMS("SAD: wrong CEA revision\n"); - return -ENOTSUPP; + return 0; } if (cea_db_offsets(cea, &start, &end)) { -- Jean Delvare SUSE L3 Support ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: linux-next: Tree for Sep 4 (amd/display/)
On 9/4/19 6:34 AM, Stephen Rothwell wrote: > Hi all, > > News: this will be the last linux-next I will release until Sept 30. > > Changes since 20190903: > on x86_64: In file included from ../drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn20/display_rq_dlg_calc_20v2.c:77:0: ../drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn20/../dml_inline_defs.h: In function ‘dml_min’: ../drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn20/../dml_inline_defs.h:34:1: error: SSE register return with SSE disabled -- ~Randy ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [EXT] Re: [v2 1/3] drm/arm/mali-dp: Add display QoS interface configuration for Mali DP500
> -Original Message- > From: Liviu Dudau > Sent: 2019年9月5日 0:13 > To: Wen He > Cc: dri-devel@lists.freedesktop.org; linux-ker...@vger.kernel.org; > brian.star...@arm.com; airl...@linux.ie; dan...@ffwll.ch; Leo Li > > Subject: Re: [EXT] Re: [v2 1/3] drm/arm/mali-dp: Add display QoS interface > configuration for Mali DP500 > > Caution: EXT Email > > On Thu, Aug 15, 2019 at 11:14:17AM +, Wen He wrote: > > > > > > > -Original Message- > > > From: Liviu Dudau > > > Sent: 2019年7月22日 17:33 > > > To: Wen He > > > Cc: dri-devel@lists.freedesktop.org; linux-ker...@vger.kernel.org; > > > brian.star...@arm.com; airl...@linux.ie; dan...@ffwll.ch; Leo Li > > > > > > Subject: Re: [EXT] Re: [v2 1/3] drm/arm/mali-dp: Add display QoS > > > interface configuration for Mali DP500 > > > > > > Caution: EXT Email > > > > > > On Mon, Jul 22, 2019 at 02:12:08AM +, Wen He wrote: > > > > > > > > > > > > > -Original Message- > > > > > From: Liviu Dudau > > > > > Sent: 2019年7月19日 19:46 > > > > > To: Wen He > > > > > Cc: dri-devel@lists.freedesktop.org; > > > > > linux-ker...@vger.kernel.org; brian.star...@arm.com; > > > > > airl...@linux.ie; dan...@ffwll.ch; Leo Li > > > > > Subject: [EXT] Re: [v2 1/3] drm/arm/mali-dp: Add display QoS > > > > > interface configuration for Mali DP500 > > > > > > > > > > Caution: EXT Email > > > > > > > > > > On Fri, Jul 19, 2019 at 05:54:45PM +0800, Wen He wrote: > > > > > > Configure the display Quality of service (QoS) levels priority > > > > > > if the optional property node "arm,malidp-aqros-value" is > > > > > > defined in DTS > > > file. > > > > > > > > > > > > QoS signaling using AQROS and AWQOS AXI interface signals, the > > > > > > AQROS is driven from the "RQOS" register, so needed to program > > > > > > the RQOS register to avoid the 4k resolution flicker issue on > > > > > > the LS1028A > > > platform. > > > > > > > > > > > > Signed-off-by: Wen He > > > > > > --- > > > > > > change in v2: > > > > > > - modify some content based on feedback from > > > > > > maintainers > > > > > > > > > > > > drivers/gpu/drm/arm/malidp_drv.c | 6 ++ > > > > > > drivers/gpu/drm/arm/malidp_hw.c | 13 + > > > > > > drivers/gpu/drm/arm/malidp_hw.h | 3 +++ > > > > > > drivers/gpu/drm/arm/malidp_regs.h | 10 ++ > > > > > > 4 files changed, 32 insertions(+) > > > > > > > > > > > > diff --git a/drivers/gpu/drm/arm/malidp_drv.c > > > > > > b/drivers/gpu/drm/arm/malidp_drv.c > > > > > > index f25ec4382277..61c49a0668a7 100644 > > > > > > --- a/drivers/gpu/drm/arm/malidp_drv.c > > > > > > +++ b/drivers/gpu/drm/arm/malidp_drv.c > > > > > > @@ -818,6 +818,12 @@ static int malidp_bind(struct device > > > > > > *dev) > > > > > > > > > > > > malidp->core_id = version; > > > > > > > > > > > > + ret = of_property_read_u32(dev->of_node, > > > > > > + > "arm,malidp-arqos-value", > > > > > > + &hwdev->arqos_value); > > > > > > + if (ret) > > > > > > + hwdev->arqos_value = 0x0; > > > > > > > > > > Is zero the default value that you want? I thought it was 0x00010001. > > > > > > > > Actually, the register default value always is 0x00010001(can be > > > > found in RM > > > document). > > > > > > Exactly, but with your code you are overwriting it to 0 if the DT > > > doesn't have the arm,malidp-arqos-value property. > > > > > > > > > > > > > > > > > > + > > > > > > /* set the number of lines used for output of RGB data */ > > > > > > ret = of_property_read_u8_array(dev->of_node, > > > > > > > > > > > > "arm,malidp-output-port-lines", diff --git > > > > > > a/drivers/gpu/drm/arm/malidp_hw.c > > > > > > b/drivers/gpu/drm/arm/malidp_hw.c index > > > > > > 50af399d7f6f..323683b1e9f7 > > > > > > 100644 > > > > > > --- a/drivers/gpu/drm/arm/malidp_hw.c > > > > > > +++ b/drivers/gpu/drm/arm/malidp_hw.c > > > > > > @@ -374,6 +374,19 @@ static void malidp500_modeset(struct > > > > > malidp_hw_device *hwdev, struct videomode * > > > > > > malidp_hw_setbits(hwdev, > > > > > > MALIDP_DISP_FUNC_ILACED, > > > > > MALIDP_DE_DISPLAY_FUNC); > > > > > > else > > > > > > malidp_hw_clearbits(hwdev, > > > MALIDP_DISP_FUNC_ILACED, > > > > > > MALIDP_DE_DISPLAY_FUNC); > > > > > > + > > > > > > + /* > > > > > > + * Program the RQoS register to avoid 4k resolution flicker > > > > > > + * on the LS1028A. > > > > > > + */ > > > > > > + if (hwdev->arqos_value) { > > > > > > + val = hwdev->arqos_value; > > > > > > + > > > > > > + if (mode->pixelclock == 59400) > > > > > > > > > > If I remember correctly, you declare the pixelclocks in the > > > > > device tree, so I wonder if this is needed here. We should just > > > > > set what value was in the DT regardless of the pixelclock, and > > > > > then you manipulate the DT to choose one of your fixed > > > > > resolutions and also set the > > > QoS value. > > > > > > > > Ye
[PATCH v2 2/3] drm/radeon: be quiet when no SAD block is found
It is fine for displays without audio functionality to not provide any SAD block in their EDID. Do not log an error in that case, just return quietly. Inspired by a similar fix to the amdgpu driver in the context of bug fdo#107825: https://bugs.freedesktop.org/show_bug.cgi?id=107825 Signed-off-by: Jean Delvare Cc: Alex Deucher Cc: "Christian König" Cc: "David (ChunMing) Zhou" Cc: David Airlie Cc: Daniel Vetter --- Changes since v1: * Fixed subject drivers/gpu/drm/radeon/radeon_audio.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) --- linux-5.2.orig/drivers/gpu/drm/radeon/radeon_audio.c2019-08-30 18:04:15.125056697 +0200 +++ linux-5.2/drivers/gpu/drm/radeon/radeon_audio.c 2019-08-30 18:04:35.078311347 +0200 @@ -367,10 +367,10 @@ static void radeon_audio_write_sad_regs( return; sad_count = drm_edid_to_sad(radeon_connector_edid(connector), &sads); - if (sad_count <= 0) { + if (sad_count < 0) DRM_ERROR("Couldn't read SADs: %d\n", sad_count); + if (sad_count <= 0) return; - } BUG_ON(!sads); if (radeon_encoder->audio && radeon_encoder->audio->write_sad_regs) -- Jean Delvare SUSE L3 Support ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v18 15/15] selftests, arm64: add a selftest for passing tagged pointers to kernel
On Fri, Aug 23, 2019 at 7:49 PM Cristian Marussi wrote: > > > Hi > > On 23/08/2019 18:16, Andrey Konovalov wrote: > > On Fri, Aug 23, 2019 at 3:56 PM Cristian Marussi > > wrote: > >> > >> Hi Andrey > >> > >> On 24/06/2019 15:33, Andrey Konovalov wrote: > >>> This patch is a part of a series that extends kernel ABI to allow to pass > >>> tagged user pointers (with the top byte set to something else other than > >>> 0x00) as syscall arguments. > >>> > >>> This patch adds a simple test, that calls the uname syscall with a > >>> tagged user pointer as an argument. Without the kernel accepting tagged > >>> user pointers the test fails with EFAULT. > >>> > >>> Signed-off-by: Andrey Konovalov > >>> --- > >>> tools/testing/selftests/arm64/.gitignore | 1 + > >>> tools/testing/selftests/arm64/Makefile| 11 +++ > >>> .../testing/selftests/arm64/run_tags_test.sh | 12 > >>> tools/testing/selftests/arm64/tags_test.c | 29 +++ > >>> 4 files changed, 53 insertions(+) > >>> create mode 100644 tools/testing/selftests/arm64/.gitignore > >>> create mode 100644 tools/testing/selftests/arm64/Makefile > >>> create mode 100755 tools/testing/selftests/arm64/run_tags_test.sh > >>> create mode 100644 tools/testing/selftests/arm64/tags_test.c > >> > >> After building a fresh Kernel from arm64/for-next-core from scratch at: > >> > >> commit 239ab658bea3b387424501e7c416640d6752dc0c > >> Merge: 6bfa3134bd3a 42d038c4fb00 1243cb6a676f d55c5f28afaf d06fa5a118f1 > >> 34b5560db40d > >> Author: Will Deacon > >> Date: Thu Aug 22 18:23:53 2019 +0100 > >> > >> Merge branches 'for-next/error-injection', 'for-next/tbi', > >> 'for-next/psci-cpuidle', 'for-next/cpu-topology' and 'for-next/52-bit-kva' > >> into for-next/core > >> > >> > >> KSFT arm64 tests build is broken for me, both setting or not KBUILD_OUTPUT= > >> > >> 13:30 $ make TARGETS=arm64 kselftest-clean > >> make[1]: Entering directory '/home/crimar01/ARM/dev/src/pdsw/out_linux' > >> rm -f -r > >> /home/crimar01/ARM/dev/src/pdsw/out_linux//kselftest/arm64/tags_test > >> make[1]: Leaving directory '/home/crimar01/ARM/dev/src/pdsw/out_linux' > >> > >> ✔ ~/ARM/dev/src/pdsw/linux [arm64_for_next_core|…8⚑ 23] > >> > >> 13:30 $ make TARGETS=arm64 kselftest > >> make[1]: Entering directory '/home/crimar01/ARM/dev/src/pdsw/out_linux' > >> arch/arm64/Makefile:56: CROSS_COMPILE_COMPAT not defined or empty, the > >> compat vDSO will not be built > >> make --no-builtin-rules INSTALL_HDR_PATH=$BUILD/usr \ > >> ARCH=arm64 -C ../../.. headers_install > >> HOSTCC scripts/basic/fixdep > >> HOSTCC scripts/unifdef > >> ... > >> ... > >> HDRINST usr/include/asm/msgbuf.h > >> HDRINST usr/include/asm/shmbuf.h > >> INSTALL /home/crimar01/ARM/dev/src/pdsw/out_linux//kselftest/usr/include > >> /opt/toolchains/gcc-arm-8.3-2019.03-x86_64-aarch64-linux-gnu/bin/aarch64-linux-gnu-gcc > >> tags_test.c -o > >> /home/crimar01/ARM/dev/src/pdsw/out_linux//kselftest/arm64/tags_test > >> tags_test.c: In function ‘main’: > >> tags_test.c:21:12: error: ‘PR_SET_TAGGED_ADDR_CTRL’ undeclared (first use > >> in this function); did you mean ‘PR_GET_TID_ADDRESS’? > >> if (prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE, 0, 0, 0) == 0) > >> ^~~ > >> PR_GET_TID_ADDRESS > >> tags_test.c:21:12: note: each undeclared identifier is reported only once > >> for each function it appears in > >> tags_test.c:21:37: error: ‘PR_TAGGED_ADDR_ENABLE’ undeclared (first use in > >> this function); did you mean ‘PR_GET_DUMPABLE’? > >> if (prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE, 0, 0, 0) == 0) > >> ^ > >> PR_GET_DUMPABLE > >> ../lib.mk:138: recipe for target > >> '/home/crimar01/ARM/dev/src/pdsw/out_linux//kselftest/arm64/tags_test' > >> failed > >> make[3]: *** > >> [/home/crimar01/ARM/dev/src/pdsw/out_linux//kselftest/arm64/tags_test] > >> Error 1 > >> Makefile:136: recipe for target 'all' failed > >> make[2]: *** [all] Error 2 > >> /home/crimar01/ARM/dev/src/pdsw/linux/Makefile:1237: recipe for target > >> 'kselftest' failed > >> make[1]: *** [kselftest] Error 2 > >> make[1]: Leaving directory '/home/crimar01/ARM/dev/src/pdsw/out_linux' > >> Makefile:179: recipe for target 'sub-make' failed > >> make: *** [sub-make] Error 2 > >> > >> Despite seeing KSFT installing Kernel Headers, they cannot be found. > >> > >> Fixing this patch like this make it work for me: > >> > >> diff --git a/tools/testing/selftests/arm64/Makefile > >> b/tools/testing/selftests/arm64/Makefile > >> index a61b2e743e99..f9f79fb272f0 100644 > >> --- a/tools/testing/selftests/arm64/Makefile > >> +++ b/tools/testing/selftests/arm64/Makefile > >> @@ -4,6 +4,7 @@ > >> ARCH ?= $(shell uname -m 2>/dev/null || echo not) > >> > >> ifneq (,$(filter $(ARCH),aarch64 arm64)) > >> +CFLAGS += -I../../../../usr/include/ > >> TEST
[PATCH v2 1/3] drm/amd: be quiet when no SAD block is found
It is fine for displays without audio functionality to not provide any SAD block in their EDID. Do not log an error in that case, just return quietly. This fixes half of bug fdo#107825: https://bugs.freedesktop.org/show_bug.cgi?id=107825 Signed-off-by: Jean Delvare Cc: Alex Deucher Cc: "Christian König" Cc: "David (ChunMing) Zhou" Cc: David Airlie Cc: Daniel Vetter Cc: Harry Wentland Cc: Leo Li --- No change since v1. drivers/gpu/drm/amd/amdgpu/dce_v10_0.c|4 ++-- drivers/gpu/drm/amd/amdgpu/dce_v11_0.c|4 ++-- drivers/gpu/drm/amd/amdgpu/dce_v6_0.c |4 ++-- drivers/gpu/drm/amd/amdgpu/dce_v8_0.c |4 ++-- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c |7 +++ 5 files changed, 11 insertions(+), 12 deletions(-) --- linux-5.2.orig/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c 2019-07-08 00:41:56.0 +0200 +++ linux-5.2/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c2019-08-30 14:28:46.081682223 +0200 @@ -1345,10 +1345,10 @@ static void dce_v10_0_audio_write_sad_re } sad_count = drm_edid_to_sad(amdgpu_connector_edid(connector), &sads); - if (sad_count <= 0) { + if (sad_count < 0) DRM_ERROR("Couldn't read SADs: %d\n", sad_count); + if (sad_count <= 0) return; - } BUG_ON(!sads); for (i = 0; i < ARRAY_SIZE(eld_reg_to_type); i++) { --- linux-5.2.orig/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c 2019-07-08 00:41:56.0 +0200 +++ linux-5.2/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c2019-08-30 14:29:27.276205310 +0200 @@ -1371,10 +1371,10 @@ static void dce_v11_0_audio_write_sad_re } sad_count = drm_edid_to_sad(amdgpu_connector_edid(connector), &sads); - if (sad_count <= 0) { + if (sad_count < 0) DRM_ERROR("Couldn't read SADs: %d\n", sad_count); + if (sad_count <= 0) return; - } BUG_ON(!sads); for (i = 0; i < ARRAY_SIZE(eld_reg_to_type); i++) { --- linux-5.2.orig/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c2019-07-08 00:41:56.0 +0200 +++ linux-5.2/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c 2019-08-30 17:58:53.613953458 +0200 @@ -1248,10 +1248,10 @@ static void dce_v6_0_audio_write_sad_reg } sad_count = drm_edid_to_sad(amdgpu_connector_edid(connector), &sads); - if (sad_count <= 0) { + if (sad_count < 0) DRM_ERROR("Couldn't read SADs: %d\n", sad_count); + if (sad_count <= 0) return; - } for (i = 0; i < ARRAY_SIZE(eld_reg_to_type); i++) { u32 tmp = 0; --- linux-5.2.orig/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c2019-07-08 00:41:56.0 +0200 +++ linux-5.2/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c 2019-08-30 14:29:01.948883708 +0200 @@ -1298,10 +1298,10 @@ static void dce_v8_0_audio_write_sad_reg } sad_count = drm_edid_to_sad(amdgpu_connector_edid(connector), &sads); - if (sad_count <= 0) { + if (sad_count < 0) DRM_ERROR("Couldn't read SADs: %d\n", sad_count); + if (sad_count <= 0) return; - } BUG_ON(!sads); for (i = 0; i < ARRAY_SIZE(eld_reg_to_type); i++) { --- linux-5.2.orig/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c 2019-07-08 00:41:56.0 +0200 +++ linux-5.2/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c 2019-08-30 14:31:03.086421910 +0200 @@ -98,11 +98,10 @@ enum dc_edid_status dm_helpers_parse_edi (struct edid *) edid->raw_edid); sad_count = drm_edid_to_sad((struct edid *) edid->raw_edid, &sads); - if (sad_count <= 0) { - DRM_INFO("SADs count is: %d, don't need to read it\n", - sad_count); + if (sad_count < 0) + DRM_ERROR("Couldn't read SADs: %d\n", sad_count); + if (sad_count <= 0) return result; - } edid_caps->audio_mode_count = sad_count < DC_MAX_AUDIO_DESC_COUNT ? sad_count : DC_MAX_AUDIO_DESC_COUNT; for (i = 0; i < edid_caps->audio_mode_count; ++i) { -- Jean Delvare SUSE L3 Support ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 0/3] ast, mgag200: Map console BO while it's being displayed
On Wed, 04 Sep 2019, Daniel Vetter wrote: I'm also not sure whether we have a real problem here, it's just debug noise that we're fighting here? It is non stop debug noise as the memory range in question is being added + deleted over and over. I doubt we want to be burning cycles like this. Thanks, Davidlohr ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Xorg indefinitely hangs in kernelspace
On Tue, Aug 06, 2019 at 09:00:10PM +0300, Jaak Ristioja wrote: > Hello! > > I'm writing to report a crash in the QXL / DRM code in the Linux kernel. > I originally filed the issue on LaunchPad and more details can be found > there, although I doubt whether these details are useful. Any change with kernel 5.3-rc7 ? cheers, Gerd
Re: [BACKPORT 4.14.y 1/8] drm/i915/fbdev: Actually configure untiled displays
On Wed, 4 Sep 2019 at 21:19, Jani Nikula wrote: > > On Tue, 03 Sep 2019, Baolin Wang wrote: > > From: Chris Wilson > > > > If we skipped all the connectors that were not part of a tile, we would > > leave conn_seq=0 and conn_configured=0, convincing ourselves that we > > had stagnated in our configuration attempts. Avoid this situation by > > starting conn_seq=ALL_CONNECTORS, and repeating until we find no more > > connectors to configure. > > > > Fixes: 754a76591b12 ("drm/i915/fbdev: Stop repeating tile configuration on > > stagnation") > > Reported-by: Maarten Lankhorst > > Signed-off-by: Chris Wilson > > Cc: Maarten Lankhorst > > Reviewed-by: Maarten Lankhorst > > Link: > > https://patchwork.freedesktop.org/patch/msgid/20190215123019.32283-1-ch...@chris-wilson.co.uk > > Cc: # v3.19+ > > Signed-off-by: Baolin Wang > > Please look into the scripts to avoid picking up stuff that has > subsequently been reverted: I am very sorry, I missed this patch had been reverted, I will check why this revert patch was not in our product kernel. Thanks for your comments. > > commit 9fa246256e09dc30820524401cdbeeaadee94025 > Author: Dave Airlie > Date: Wed Apr 24 10:47:56 2019 +1000 > > Revert "drm/i915/fbdev: Actually configure untiled displays" > > This reverts commit d179b88deb3bf6fed4991a31fd6f0f2cad21fab5. > > This commit is documented to break userspace X.org modesetting driver in > certain configurations. > > The X.org modesetting userspace driver is broken. No fixes are available > yet. In order for this patch to be applied it either needs a config option or > a workaround developed. > > This has been reported a few times, saying it's a userspace problem is > clearly against the regression rules. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=109806 > Signed-off-by: Dave Airlie > Cc: # v3.19+ > > > > BR, > Jani. > > > > --- > > drivers/gpu/drm/i915/intel_fbdev.c | 12 +++- > > 1 file changed, 7 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_fbdev.c > > b/drivers/gpu/drm/i915/intel_fbdev.c > > index da2d309..14eb8a0 100644 > > --- a/drivers/gpu/drm/i915/intel_fbdev.c > > +++ b/drivers/gpu/drm/i915/intel_fbdev.c > > @@ -326,8 +326,8 @@ static bool intel_fb_initial_config(struct > > drm_fb_helper *fb_helper, > > bool *enabled, int width, int height) > > { > > struct drm_i915_private *dev_priv = to_i915(fb_helper->dev); > > - unsigned long conn_configured, conn_seq, mask; > > unsigned int count = min(fb_helper->connector_count, BITS_PER_LONG); > > + unsigned long conn_configured, conn_seq; > > int i, j; > > bool *save_enabled; > > bool fallback = true, ret = true; > > @@ -345,10 +345,9 @@ static bool intel_fb_initial_config(struct > > drm_fb_helper *fb_helper, > > drm_modeset_backoff(&ctx); > > > > memcpy(save_enabled, enabled, count); > > - mask = GENMASK(count - 1, 0); > > + conn_seq = GENMASK(count - 1, 0); > > conn_configured = 0; > > retry: > > - conn_seq = conn_configured; > > for (i = 0; i < count; i++) { > > struct drm_fb_helper_connector *fb_conn; > > struct drm_connector *connector; > > @@ -361,7 +360,8 @@ static bool intel_fb_initial_config(struct > > drm_fb_helper *fb_helper, > > if (conn_configured & BIT(i)) > > continue; > > > > - if (conn_seq == 0 && !connector->has_tile) > > + /* First pass, only consider tiled connectors */ > > + if (conn_seq == GENMASK(count - 1, 0) && !connector->has_tile) > > continue; > > > > if (connector->status == connector_status_connected) > > @@ -465,8 +465,10 @@ static bool intel_fb_initial_config(struct > > drm_fb_helper *fb_helper, > > conn_configured |= BIT(i); > > } > > > > - if ((conn_configured & mask) != mask && conn_configured != conn_seq) > > + if (conn_configured != conn_seq) { /* repeat until no more are found > > */ > > + conn_seq = conn_configured; > > goto retry; > > + } > > > > /* > >* If the BIOS didn't enable everything it could, fall back to have > > the > > -- > Jani Nikula, Intel Open Source Graphics Center -- Baolin Wang Best Regards ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 0/3] drm/edid: don't log errors on absent or old CEA SAD blocks
Hi all, This is my attempt to fix bug fdo#107825: https://bugs.freedesktop.org/show_bug.cgi?id=107825 [PATCH 1/3] drm/amd: be quiet when no SAD block is found [PATCH 2/3] drm/radeon: be quiet when no SAD block is found [PATCH 3/3] drm/edid: no CEA extension is not an error Changes since v1: * Fixed subject of patch 2 * Treat CEA extension version < 3 as non-error too (suggested by Ville Syrjälä) -- Jean Delvare SUSE L3 Support ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v18 15/15] selftests, arm64: add a selftest for passing tagged pointers to kernel
Hi Andrey ! On 04/09/2019 15:52, Andrey Konovalov wrote: > On Fri, Aug 23, 2019 at 7:49 PM Cristian Marussi > wrote: >> >> >> Hi >> >> On 23/08/2019 18:16, Andrey Konovalov wrote: >>> On Fri, Aug 23, 2019 at 3:56 PM Cristian Marussi >>> wrote: Hi Andrey On 24/06/2019 15:33, Andrey Konovalov wrote: > This patch is a part of a series that extends kernel ABI to allow to pass > tagged user pointers (with the top byte set to something else other than > 0x00) as syscall arguments. > > This patch adds a simple test, that calls the uname syscall with a > tagged user pointer as an argument. Without the kernel accepting tagged > user pointers the test fails with EFAULT. > > Signed-off-by: Andrey Konovalov > --- > tools/testing/selftests/arm64/.gitignore | 1 + > tools/testing/selftests/arm64/Makefile| 11 +++ > .../testing/selftests/arm64/run_tags_test.sh | 12 > tools/testing/selftests/arm64/tags_test.c | 29 +++ > 4 files changed, 53 insertions(+) > create mode 100644 tools/testing/selftests/arm64/.gitignore > create mode 100644 tools/testing/selftests/arm64/Makefile > create mode 100755 tools/testing/selftests/arm64/run_tags_test.sh > create mode 100644 tools/testing/selftests/arm64/tags_test.c After building a fresh Kernel from arm64/for-next-core from scratch at: commit 239ab658bea3b387424501e7c416640d6752dc0c Merge: 6bfa3134bd3a 42d038c4fb00 1243cb6a676f d55c5f28afaf d06fa5a118f1 34b5560db40d Author: Will Deacon Date: Thu Aug 22 18:23:53 2019 +0100 Merge branches 'for-next/error-injection', 'for-next/tbi', 'for-next/psci-cpuidle', 'for-next/cpu-topology' and 'for-next/52-bit-kva' into for-next/core KSFT arm64 tests build is broken for me, both setting or not KBUILD_OUTPUT= 13:30 $ make TARGETS=arm64 kselftest-clean make[1]: Entering directory '/home/crimar01/ARM/dev/src/pdsw/out_linux' rm -f -r /home/crimar01/ARM/dev/src/pdsw/out_linux//kselftest/arm64/tags_test make[1]: Leaving directory '/home/crimar01/ARM/dev/src/pdsw/out_linux' ✔ ~/ARM/dev/src/pdsw/linux [arm64_for_next_core|…8⚑ 23] 13:30 $ make TARGETS=arm64 kselftest make[1]: Entering directory '/home/crimar01/ARM/dev/src/pdsw/out_linux' arch/arm64/Makefile:56: CROSS_COMPILE_COMPAT not defined or empty, the compat vDSO will not be built make --no-builtin-rules INSTALL_HDR_PATH=$BUILD/usr \ ARCH=arm64 -C ../../.. headers_install HOSTCC scripts/basic/fixdep HOSTCC scripts/unifdef ... ... HDRINST usr/include/asm/msgbuf.h HDRINST usr/include/asm/shmbuf.h INSTALL /home/crimar01/ARM/dev/src/pdsw/out_linux//kselftest/usr/include /opt/toolchains/gcc-arm-8.3-2019.03-x86_64-aarch64-linux-gnu/bin/aarch64-linux-gnu-gcc tags_test.c -o /home/crimar01/ARM/dev/src/pdsw/out_linux//kselftest/arm64/tags_test tags_test.c: In function ‘main’: tags_test.c:21:12: error: ‘PR_SET_TAGGED_ADDR_CTRL’ undeclared (first use in this function); did you mean ‘PR_GET_TID_ADDRESS’? if (prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE, 0, 0, 0) == 0) ^~~ PR_GET_TID_ADDRESS tags_test.c:21:12: note: each undeclared identifier is reported only once for each function it appears in tags_test.c:21:37: error: ‘PR_TAGGED_ADDR_ENABLE’ undeclared (first use in this function); did you mean ‘PR_GET_DUMPABLE’? if (prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE, 0, 0, 0) == 0) ^ PR_GET_DUMPABLE ../lib.mk:138: recipe for target '/home/crimar01/ARM/dev/src/pdsw/out_linux//kselftest/arm64/tags_test' failed make[3]: *** [/home/crimar01/ARM/dev/src/pdsw/out_linux//kselftest/arm64/tags_test] Error 1 Makefile:136: recipe for target 'all' failed make[2]: *** [all] Error 2 /home/crimar01/ARM/dev/src/pdsw/linux/Makefile:1237: recipe for target 'kselftest' failed make[1]: *** [kselftest] Error 2 make[1]: Leaving directory '/home/crimar01/ARM/dev/src/pdsw/out_linux' Makefile:179: recipe for target 'sub-make' failed make: *** [sub-make] Error 2 Despite seeing KSFT installing Kernel Headers, they cannot be found. Fixing this patch like this make it work for me: diff --git a/tools/testing/selftests/arm64/Makefile b/tools/testing/selftests/arm64/Makefile index a61b2e743e99..f9f79fb272f0 100644 --- a/tools/testing/selftests/arm64/Makefile +++ b/tools/testing/selftests/arm64/Makefile @@ -4,6 +4,7 @@ ARCH ?= $(shell uname -m 2>/dev/null || echo not) ifneq (,$(filter $(ARCH),aa
Re: linux-next: manual merge of the drm tree with the kbuild tree
On Tue, Sep 3, 2019 at 11:46 PM Stephen Rothwell wrote: > > Hi all, > > Today's linux-next merge of the drm tree got conflicts in: > > drivers/gpu/drm/amd/display/dc/calcs/Makefile > drivers/gpu/drm/amd/display/dc/dml/Makefile > drivers/gpu/drm/amd/display/dc/dsc/Makefile > > between commit: > > 30851871d5ab ("kbuild: change *FLAGS_.o to take the path > relative to $(obj)") > > from the kbuild tree and commit: > > 0f0727d971f6 ("drm/amd/display: readd -msse2 to prevent Clang from emitting > libcalls to undefined SW FP routines") > > from the drm tree. > > I fixed it up (see below) and can carry the fix as necessary. This > is now fixed as far as linux-next is concerned, but any non trivial > conflicts should be mentioned to your upstream maintainer when your tree > is submitted for merging. You may also want to consider cooperating > with the maintainer of the conflicting tree to minimise any particularly > complex conflicts. My changes LGTM, thanks! > > -- > Cheers, > Stephen Rothwell > > diff --cc drivers/gpu/drm/amd/display/dc/calcs/Makefile > index d930df63772c,16614d73a5fc.. > --- a/drivers/gpu/drm/amd/display/dc/calcs/Makefile > +++ b/drivers/gpu/drm/amd/display/dc/calcs/Makefile > @@@ -32,9 -32,13 +32,13 @@@ endi > > calcs_ccflags := -mhard-float -msse $(cc_stack_align) > > + ifdef CONFIG_CC_IS_CLANG > + calcs_ccflags += -msse2 > + endif > + > -CFLAGS_dcn_calcs.o := $(calcs_ccflags) > -CFLAGS_dcn_calc_auto.o := $(calcs_ccflags) > -CFLAGS_dcn_calc_math.o := $(calcs_ccflags) -Wno-tautological-compare > +CFLAGS_$(AMDDALPATH)/dc/calcs/dcn_calcs.o := $(calcs_ccflags) > +CFLAGS_$(AMDDALPATH)/dc/calcs/dcn_calc_auto.o := $(calcs_ccflags) > +CFLAGS_$(AMDDALPATH)/dc/calcs/dcn_calc_math.o := $(calcs_ccflags) > -Wno-tautological-compare > > BW_CALCS = dce_calcs.o bw_fixed.o custom_float.o > > diff --cc drivers/gpu/drm/amd/display/dc/dml/Makefile > index 83792e2c0f0e,95fd2beca80c.. > --- a/drivers/gpu/drm/amd/display/dc/dml/Makefile > +++ b/drivers/gpu/drm/amd/display/dc/dml/Makefile > @@@ -32,16 -32,25 +32,20 @@@ endi > > dml_ccflags := -mhard-float -msse $(cc_stack_align) > > + ifdef CONFIG_CC_IS_CLANG > + dml_ccflags += -msse2 > + endif > + > -CFLAGS_display_mode_lib.o := $(dml_ccflags) > +CFLAGS_$(AMDDALPATH)/dc/dml/display_mode_lib.o := $(dml_ccflags) > > ifdef CONFIG_DRM_AMD_DC_DCN2_0 > -CFLAGS_display_mode_vba.o := $(dml_ccflags) > -CFLAGS_display_mode_vba_20.o := $(dml_ccflags) > -CFLAGS_display_rq_dlg_calc_20.o := $(dml_ccflags) > -CFLAGS_display_mode_vba_20v2.o := $(dml_ccflags) > -CFLAGS_display_rq_dlg_calc_20v2.o := $(dml_ccflags) > +CFLAGS_$(AMDDALPATH)/dc/dml/display_mode_vba.o := $(dml_ccflags) > +CFLAGS_$(AMDDALPATH)/dc/dml/dcn20/display_mode_vba_20.o := $(dml_ccflags) > +CFLAGS_$(AMDDALPATH)/dc/dml/dcn20/display_rq_dlg_calc_20.o := $(dml_ccflags) > endif > -ifdef CONFIG_DRM_AMD_DCN3AG > -CFLAGS_display_mode_vba_3ag.o := $(dml_ccflags) > -endif > -CFLAGS_dml1_display_rq_dlg_calc.o := $(dml_ccflags) > -CFLAGS_display_rq_dlg_helpers.o := $(dml_ccflags) > -CFLAGS_dml_common_defs.o := $(dml_ccflags) > +CFLAGS_$(AMDDALPATH)/dc/dml/dml1_display_rq_dlg_calc.o := $(dml_ccflags) > +CFLAGS_$(AMDDALPATH)/dc/dml/display_rq_dlg_helpers.o := $(dml_ccflags) > +CFLAGS_$(AMDDALPATH)/dc/dml/dml_common_defs.o := $(dml_ccflags) > > DML = display_mode_lib.o display_rq_dlg_helpers.o > dml1_display_rq_dlg_calc.o \ > dml_common_defs.o > diff --cc drivers/gpu/drm/amd/display/dc/dsc/Makefile > index c3922d6e7696,17db603f2d1f.. > --- a/drivers/gpu/drm/amd/display/dc/dsc/Makefile > +++ b/drivers/gpu/drm/amd/display/dc/dsc/Makefile > @@@ -9,9 -9,14 +9,13 @@@ endi > > dsc_ccflags := -mhard-float -msse $(cc_stack_align) > > + ifdef CONFIG_CC_IS_CLANG > + dsc_ccflags += -msse2 > + endif > + > -CFLAGS_rc_calc.o := $(dsc_ccflags) > -CFLAGS_rc_calc_dpi.o := $(dsc_ccflags) > -CFLAGS_codec_main_amd.o := $(dsc_ccflags) > -CFLAGS_dc_dsc.o := $(dsc_ccflags) > +CFLAGS_$(AMDDALPATH)/dc/dsc/rc_calc.o := $(dsc_ccflags) > +CFLAGS_$(AMDDALPATH)/dc/dsc/rc_calc_dpi.o := $(dsc_ccflags) > +CFLAGS_$(AMDDALPATH)/dc/dsc/dc_dsc.o := $(dsc_ccflags) > > DSC = dc_dsc.o rc_calc.o rc_calc_dpi.o > -- Thanks, ~Nick Desaulniers ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5, 32/32] drm/mediatek: add support for mediatek SOC MT8183
Hi, Yongqiang: On Thu, 2019-08-29 at 22:50 +0800, yongqiang@mediatek.com wrote: > From: Yongqiang Niu > > This patch add support for mediatek SOC MT8183 > 1.ovl_2l share driver with ovl > 2.rdma1 share drive with rdma0, but fifo size is different > 3.add mt8183 mutex private data, and mmsys private data > 4.add mt8183 main and external path module for crtc create > > Signed-off-by: Yongqiang Niu > --- > drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 18 + > drivers/gpu/drm/mediatek/mtk_disp_rdma.c | 27 - > drivers/gpu/drm/mediatek/mtk_drm_ddp.c | 69 > > drivers/gpu/drm/mediatek/mtk_drm_ddp.h | 1 + > drivers/gpu/drm/mediatek/mtk_drm_drv.c | 47 ++ > 5 files changed, 161 insertions(+), 1 deletion(-) > [snip] > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c > b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c > index 9a6f0a2..24945fe 100644 > --- a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c > +++ b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c > @@ -62,6 +62,7 @@ struct mtk_disp_rdma { > struct mtk_ddp_comp ddp_comp; > struct drm_crtc *crtc; > const struct mtk_disp_rdma_data *data; > + u32 fifo_size; > }; > > static inline struct mtk_disp_rdma *comp_to_rdma(struct mtk_ddp_comp *comp) > @@ -130,10 +131,16 @@ static void mtk_rdma_config(struct mtk_ddp_comp *comp, > unsigned int width, > unsigned int threshold; > unsigned int reg; > struct mtk_disp_rdma *rdma = comp_to_rdma(comp); > + u32 rdma_fifo_size; > > rdma_update_bits(comp, DISP_REG_RDMA_SIZE_CON_0, 0xfff, width); > rdma_update_bits(comp, DISP_REG_RDMA_SIZE_CON_1, 0xf, height); > > + if (rdma->fifo_size) > + rdma_fifo_size = rdma->fifo_size; > + else > + rdma_fifo_size = RDMA_FIFO_SIZE(rdma); I think the fifo size part should be an independent patch because it has no strong relation with MT8183. > + > /* >* Enable FIFO underflow since DSI and DPI can't be blocked. >* Keep the FIFO pseudo size reset default of 8 KiB. Set the > @@ -142,7 +149,7 @@ static void mtk_rdma_config(struct mtk_ddp_comp *comp, > unsigned int width, >*/ > threshold = width * height * vrefresh * 4 * 7 / 100; > reg = RDMA_FIFO_UNDERFLOW_EN | > - RDMA_FIFO_PSEUDO_SIZE(RDMA_FIFO_SIZE(rdma)) | > + RDMA_FIFO_PSEUDO_SIZE(rdma_fifo_size) | > RDMA_OUTPUT_VALID_FIFO_THRESHOLD(threshold); > writel(reg, comp->regs + DISP_REG_RDMA_FIFO_CON); > } > @@ -284,6 +291,18 @@ static int mtk_disp_rdma_probe(struct platform_device > *pdev) > return comp_id; > } > > + if (of_find_property(dev->of_node, "mediatek,rdma_fifo_size", &ret)) { > + ret = of_property_read_u32(dev->of_node, > +"mediatek,rdma_fifo_size", > +&priv->fifo_size); > + if (ret) { > + dev_err(dev, "Failed to get rdma fifo size\n"); > + return ret; > + } > + > + priv->fifo_size *= SZ_1K; > + } > + > ret = mtk_ddp_comp_init(dev, dev->of_node, &priv->ddp_comp, comp_id, > &mtk_disp_rdma_funcs); > if (ret) { > @@ -328,11 +347,17 @@ static int mtk_disp_rdma_remove(struct platform_device > *pdev) > .fifo_size = SZ_8K, > }; > [snip] > @@ -514,6 +558,7 @@ static int mtk_drm_probe(struct platform_device *pdev) >*/ > if (comp_type == MTK_DISP_COLOR || > comp_type == MTK_DISP_OVL || > + comp_type == MTK_DISP_OVL_2L || I think this should be squashed into "[v5,15/32] drm/mediatek: add commponent OVL_2L0'. Regards, CK > comp_type == MTK_DISP_RDMA || > comp_type == MTK_DSI || > comp_type == MTK_DPI) { ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 0/3] ast, mgag200: Map console BO while it's being displayed
On Thu, Sep 5, 2019 at 9:01 AM Gerd Hoffmann wrote: > > Hi, > > > - imo we should fix this by using the io_mapping stuff, that avoids > > the overhead of repeated pat checks for map/unmap. > > Another idea: IIRC ttm has a move_notify callback. So we could simply > keep mappings active even when the refcount goes down to zero. Then do > the actual unmap either in the move_notify or in the destroy callback. Yeah that should be a really clean solution, and only needs changes in the vram helpers. Which is nice, more common code! -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: gnome-shell stuck because of amdgpu driver [5.3 RC5]
On Thu, Sep 5, 2019 at 12:27 AM Mikhail Gavrilov wrote: > > On Wed, 4 Sep 2019 at 13:37, Daniel Vetter wrote: > > > > Extend your backtrac warning slightly like > > > > WARN(r, "we're stuck on fence %pS\n", fence->ops); > > > > Also adding Harry and Alex, I'm not really working on amdgpu ... > > [ 3511.998320] [ cut here ] > [ 3511.998714] we're stuck on fence > amdgpu_fence_ops+0x0/0xc220 [amdgpu]$ I think those fences are only emitted for CS, not display related. Adding Christian König. -Daniel > [ 3511.998991] WARNING: CPU: 10 PID: 1811 at > drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c:332 > amdgpu_fence_wait_empty+0x1c6/0x240 [amdgpu] > [ 3511.999009] Modules linked in: rfcomm fuse xt_CHECKSUM > xt_MASQUERADE nf_nat_tftp nf_conntrack_tftp tun bridge stp llc > nf_conntrack_netbios_ns nf_conntrack_broadcast xt_CT ip6t_REJECT > nf_reject_ipv6 ip6t_rpfilter ipt_REJECT nf_reject_ipv4 xt_conntrack > ebtable_nat ip6table_nat ip6table_mangle ip6table_raw > ip6table_security iptable_nat nf_nat iptable_mangle iptable_raw > iptable_security nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 libcrc32c > ip_set nfnetlink ebtable_filter ebtables ip6table_filter ip6_tables > iptable_filter cmac bnep sunrpc vfat fat edac_mce_amd kvm_amd > snd_hda_codec_realtek rtwpci snd_hda_codec_generic kvm ledtrig_audio > snd_hda_codec_hdmi uvcvideo rtw88 videobuf2_vmalloc snd_hda_intel > videobuf2_memops videobuf2_v4l2 irqbypass snd_usb_audio snd_hda_codec > videobuf2_common crct10dif_pclmul snd_usbmidi_lib crc32_pclmul > mac80211 snd_rawmidi videodev snd_hda_core ghash_clmulni_intel btusb > snd_hwdep btrtl snd_seq btbcm btintel snd_seq_device eeepc_wmi > bluetooth xpad joydev mc snd_pcm > [ 3511.999076] asus_wmi ff_memless cfg80211 sparse_keymap video > wmi_bmof ecdh_generic snd_timer ecc sp5100_tco k10temp snd i2c_piix4 > ccp rfkill soundcore libarc4 gpio_amdpt gpio_generic acpi_cpufreq > binfmt_misc ip_tables hid_logitech_hidpp hid_logitech_dj amdgpu > amd_iommu_v2 gpu_sched ttm drm_kms_helper drm crc32c_intel igb dca > nvme i2c_algo_bit nvme_core wmi pinctrl_amd > [ 3511.999126] CPU: 10 PID: 1811 Comm: Xorg Not tainted > 5.3.0-0.rc6.git2.1c.fc32.x86_64 #1 > [ 3511.999131] Hardware name: System manufacturer System Product > Name/ROG STRIX X470-I GAMING, BIOS 2703 08/20/2019 > [ 3511.999253] RIP: 0010:amdgpu_fence_wait_empty+0x1c6/0x240 [amdgpu] > [ 3511.999278] Code: fe ff ff 31 c0 c3 48 89 ef e8 36 29 04 cb 84 c0 > 74 08 48 89 ef e8 8a a9 21 cb 48 8b 75 08 48 c7 c7 2c 16 86 c0 e8 82 > b8 b9 ca <0f> 0b b8 ea ff ff ff 5d c3 e8 ec 57 c3 ca 84 c0 0f 85 6f ff > ff ff > [ 3511.999282] RSP: 0018:b9c04170f798 EFLAGS: 00210282 > [ 3511.999288] RAX: RBX: 8d2ce5205a80 RCX: > 0006 > [ 3511.999292] RDX: 0007 RSI: 8d2c5bea4070 RDI: > 8d2cfb5d9e00 > [ 3511.999296] RBP: 8d28becae480 R08: 0331b36fd503 R09: > > [ 3511.999299] R10: R11: R12: > 8d2ce520 > [ 3511.999303] R13: R14: R15: > 8d2ce154 > [ 3511.999308] FS: 7f59a5bc6f00() GS:8d2cfb40() > knlGS: > [ 3511.999311] CS: 0010 DS: ES: CR0: 80050033 > [ 3511.999315] CR2: 1108bc475960 CR3: 00075bf32000 CR4: > 003406e0 > [ 3511.999319] Call Trace: > [ 3511.999394] amdgpu_pm_compute_clocks+0x70/0x5f0 [amdgpu] > [ 3511.999503] dm_pp_apply_display_requirements+0x1a8/0x1c0 [amdgpu] > [ 3511.999609] dce12_update_clocks+0xd8/0x110 [amdgpu] > [ 3511.999712] dc_commit_state+0x414/0x590 [amdgpu] > [ 3511.999725] ? find_held_lock+0x32/0x90 > [ 3511.999832] amdgpu_dm_atomic_commit_tail+0xd18/0x1cf0 [amdgpu] > [ 3511.999844] ? reacquire_held_locks+0xed/0x210 > [ 3511.999859] ? ttm_eu_backoff_reservation+0xa5/0x160 [ttm] > [ 3511.999866] ? find_held_lock+0x32/0x90 > [ 3511.999872] ? find_held_lock+0x32/0x90 > [ 3511.999881] ? __lock_acquire+0x247/0x1910 > [ 3511.999893] ? find_held_lock+0x32/0x90 > [ 3511.01] ? mark_held_locks+0x50/0x80 > [ 3511.07] ? _raw_spin_unlock_irq+0x29/0x40 > [ 3511.13] ? lockdep_hardirqs_on+0xf0/0x180 > [ 3511.19] ? _raw_spin_unlock_irq+0x29/0x40 > [ 3511.24] ? wait_for_completion_timeout+0x75/0x190 > [ 3511.52] ? commit_tail+0x3c/0x70 [drm_kms_helper] > [ 3511.66] commit_tail+0x3c/0x70 [drm_kms_helper] > [ 3511.79] drm_atomic_helper_commit+0xe3/0x150 [drm_kms_helper] > [ 3512.02] drm_mode_atomic_ioctl+0x793/0x9b0 [drm] > [ 3512.14] ? __lock_acquire+0x247/0x1910 > [ 3512.44] ? drm_atomic_set_property+0xa50/0xa50 [drm] > [ 3512.66] drm_ioctl_kernel+0xaa/0xf0 [drm] > [ 3512.88] drm_ioctl+0x208/0x390 [drm] > [ 3512.000108] ? drm_atomic_set_property+0xa50/0xa50 [drm] > [ 3512.000120] ? lockdep_hardirqs_on+0xf0/0x180 > [ 3512.000205] amdgpu_drm_ioctl+0x49/0x80 [amdgpu] > [ 3512.000216] do_vfs_ioctl+0x411/0x750 > [ 3512.000229] ksys_
Re: [PATCH v2 0/3] ast, mgag200: Map console BO while it's being displayed
Hi Am 05.09.19 um 09:56 schrieb Daniel Vetter: > On Thu, Sep 5, 2019 at 9:01 AM Gerd Hoffmann wrote: >> >> Hi, >> >>> - imo we should fix this by using the io_mapping stuff, that avoids >>> the overhead of repeated pat checks for map/unmap. >> >> Another idea: IIRC ttm has a move_notify callback. So we could simply >> keep mappings active even when the refcount goes down to zero. Then do >> the actual unmap either in the move_notify or in the destroy callback. > > Yeah that should be a really clean solution, and only needs changes in > the vram helpers. Which is nice, more common code! But the console's BO is special wrt to mapping. Putting special code for console handling into vram helpers doesn't seem right. I think it's preferable to keep the special cases located in fbdev emulation. Or even better in DRM client code, so that other, future, internal clients automatically do the right thing. Best regards Thomas > -Daniel > -- Thomas Zimmermann Graphics Driver Developer SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany GF: Felix Imendörffer, Mary Higgins, Sri Rasiah HRB 21284 (AG Nürnberg) signature.asc Description: OpenPGP digital signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/panfrost: Fix regulator_get_optional() misuse
+Steven On Wed, Sep 4, 2019 at 1:30 PM Mark Brown wrote: > > The panfrost driver requests a supply using regulator_get_optional() > but both the name of the supply and the usage pattern suggest that it is > being used for the main power for the device and is not at all optional > for the device for function, there is no meaningful handling for absent > supplies. Such regulators should use the vanilla regulator_get() > interface, it will ensure that even if a supply is not described in the > system integration one will be provided in software. I guess commits e21dd290881b ("drm/panfrost: Enable devfreq to work without regulator") and c90f30812a79 ("drm/panfrost: Add missing check for pfdev->regulator") in -next should be reverted or partially reverted? Rob ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3] video: hyperv: hyperv_fb: Support deferred IO for Hyper-V frame buffer driver
Without deferred IO support, hyperv_fb driver informs the host to refresh the entire guest frame buffer at fixed rate, e.g. at 20Hz, no matter there is screen update or not. This patch supports deferred IO for screens in graphics mode and also enables the frame buffer on-demand refresh. The highest refresh rate is still set at 20Hz. Currently Hyper-V only takes a physical address from guest as the starting address of frame buffer. This implies the guest must allocate contiguous physical memory for frame buffer. In addition, Hyper-V Gen 2 VMs only accept address from MMIO region as frame buffer address. Due to these limitations on Hyper-V host, we keep a shadow copy of frame buffer in the guest. This means one more copy of the dirty rectangle inside guest when doing the on-demand refresh. This can be optimized in the future with help from host. For now the host performance gain from deferred IO outweighs the shadow copy impact in the guest. Signed-off-by: Wei Hu --- v2: Incorporated review comments from Michael Kelley - Increased dirty rectangle by one row in deferred IO case when sending to Hyper-V. - Corrected the dirty rectangle size in the text mode. - Added more comments. - Other minor code cleanups. v3: Incorporated more review comments - Removed a few unnecessary variable tests drivers/video/fbdev/Kconfig | 1 + drivers/video/fbdev/hyperv_fb.c | 216 +--- 2 files changed, 197 insertions(+), 20 deletions(-) diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig index 1b2f5f31fb6f..e781f89a1824 100644 --- a/drivers/video/fbdev/Kconfig +++ b/drivers/video/fbdev/Kconfig @@ -2241,6 +2241,7 @@ config FB_HYPERV select FB_CFB_FILLRECT select FB_CFB_COPYAREA select FB_CFB_IMAGEBLIT + select FB_DEFERRED_IO help This framebuffer driver supports Microsoft Hyper-V Synthetic Video. diff --git a/drivers/video/fbdev/hyperv_fb.c b/drivers/video/fbdev/hyperv_fb.c index 1464c6f14687..63bfd35e392c 100644 --- a/drivers/video/fbdev/hyperv_fb.c +++ b/drivers/video/fbdev/hyperv_fb.c @@ -237,6 +237,7 @@ struct synthvid_msg { #define RING_BUFSIZE (256 * 1024) #define VSP_TIMEOUT (10 * HZ) #define HVFB_UPDATE_DELAY (HZ / 20) +#define HVFB_ONDEMAND_THROTTLE (HZ / 20) struct hvfb_par { struct fb_info *info; @@ -256,6 +257,17 @@ struct hvfb_par { bool synchronous_fb; struct notifier_block hvfb_panic_nb; + + /* Memory for deferred IO and frame buffer itself */ + unsigned char *dio_vp; + unsigned char *mmio_vp; + unsigned long mmio_pp; + spinlock_t docopy_lock; /* Lock to protect memory copy */ + + /* Dirty rectangle, protected by delayed_refresh_lock */ + int x1, y1, x2, y2; + bool delayed_refresh; + spinlock_t delayed_refresh_lock; }; static uint screen_width = HVFB_WIDTH; @@ -264,6 +276,7 @@ static uint screen_width_max = HVFB_WIDTH; static uint screen_height_max = HVFB_HEIGHT; static uint screen_depth; static uint screen_fb_size; +static uint dio_fb_size; /* FB size for deferred IO */ /* Send message to Hyper-V host */ static inline int synthvid_send(struct hv_device *hdev, @@ -350,28 +363,92 @@ static int synthvid_send_ptr(struct hv_device *hdev) } /* Send updated screen area (dirty rectangle) location to host */ -static int synthvid_update(struct fb_info *info) +static int +synthvid_update(struct fb_info *info, int x1, int y1, int x2, int y2) { struct hv_device *hdev = device_to_hv_device(info->device); struct synthvid_msg msg; memset(&msg, 0, sizeof(struct synthvid_msg)); + if (x2 == INT_MAX) + x2 = info->var.xres; + if (y2 == INT_MAX) + y2 = info->var.yres; msg.vid_hdr.type = SYNTHVID_DIRT; msg.vid_hdr.size = sizeof(struct synthvid_msg_hdr) + sizeof(struct synthvid_dirt); msg.dirt.video_output = 0; msg.dirt.dirt_count = 1; - msg.dirt.rect[0].x1 = 0; - msg.dirt.rect[0].y1 = 0; - msg.dirt.rect[0].x2 = info->var.xres; - msg.dirt.rect[0].y2 = info->var.yres; + msg.dirt.rect[0].x1 = (x1 > x2) ? 0 : x1; + msg.dirt.rect[0].y1 = (y1 > y2) ? 0 : y1; + msg.dirt.rect[0].x2 = + (x2 < x1 || x2 > info->var.xres) ? info->var.xres : x2; + msg.dirt.rect[0].y2 = + (y2 < y1 || y2 > info->var.yres) ? info->var.yres : y2; synthvid_send(hdev, &msg); return 0; } +static void hvfb_docopy(struct hvfb_par *par, + unsigned long offset, + unsigned long size) +{ + if (!par || !par->mmio_vp || !par->dio_vp || !par->fb_ready || + size == 0 || offset >= dio_fb_size) + return; + + if (offset + size > dio_fb_size) + size = dio_fb_size - offset; + + memcpy(par->mmio_vp + offset, par->dio_vp + offset, size); +} +
Re: [PATCH 7/8] drm/vmwgfx: switch to own vma manager
On Thu, 2019-09-05 at 09:05 +0200, Gerd Hoffmann wrote: > Add struct drm_vma_offset_manager to vma_private, initialize it and > pass it to ttm_bo_device_init(). > > With this in place the last user of ttm's embedded vma offset manager > is gone and we can remove it (in a separate patch). > > Signed-off-by: Gerd Hoffmann > --- > drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 1 + > drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 6 +- > 2 files changed, 6 insertions(+), 1 deletion(-) > > Reviewed-by: Thomas Hellström I assume this will be merged through drm-misc? /Thomas ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH AUTOSEL 4.19 147/167] drm/panel: Add support for Armadeus ST0700 Adapt
On Tue, Sep 3, 2019 at 5:31 PM Sasha Levin wrote: > > From: Sébastien Szymanski > > [ Upstream commit c479450f61c7f1f248c9a54aedacd2a6ca521ff8 ] > > This patch adds support for the Armadeus ST0700 Adapt. It comes with a > Santek ST0700I5Y-RBSLW 7.0" WVGA (800x480) TFT and an adapter board so > that it can be connected on the TFT header of Armadeus Dev boards. > > Cc: sta...@vger.kernel.org # v4.19 > Reviewed-by: Rob Herring > Signed-off-by: Sébastien Szymanski > Signed-off-by: Sam Ravnborg > Link: > https://patchwork.freedesktop.org/patch/msgid/20190507152713.27494-1-sebastien.szyman...@armadeus.com > Signed-off-by: Sasha Levin > --- > .../display/panel/armadeus,st0700-adapt.txt | 9 ++ > drivers/gpu/drm/panel/panel-simple.c | 29 +++ > 2 files changed, 38 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/display/panel/armadeus,st0700-adapt.txt Looks like a new feature, not stable material. Not sure why it got tagged for stable. Rob ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH AUTOSEL 4.19 147/167] drm/panel: Add support for Armadeus ST0700 Adapt
On Thu, Sep 05, 2019 at 09:55:58AM +0100, Rob Herring wrote: > On Tue, Sep 3, 2019 at 5:31 PM Sasha Levin wrote: > > > > From: Sébastien Szymanski > > > > [ Upstream commit c479450f61c7f1f248c9a54aedacd2a6ca521ff8 ] > > > > This patch adds support for the Armadeus ST0700 Adapt. It comes with a > > Santek ST0700I5Y-RBSLW 7.0" WVGA (800x480) TFT and an adapter board so > > that it can be connected on the TFT header of Armadeus Dev boards. > > > > Cc: sta...@vger.kernel.org # v4.19 > > Reviewed-by: Rob Herring > > Signed-off-by: Sébastien Szymanski > > Signed-off-by: Sam Ravnborg > > Link: > > https://patchwork.freedesktop.org/patch/msgid/20190507152713.27494-1-sebastien.szyman...@armadeus.com > > Signed-off-by: Sasha Levin > > --- > > .../display/panel/armadeus,st0700-adapt.txt | 9 ++ > > drivers/gpu/drm/panel/panel-simple.c | 29 +++ > > 2 files changed, 38 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/display/panel/armadeus,st0700-adapt.txt > > Looks like a new feature, not stable material. Not sure why it got > tagged for stable. New device ids/tables are able to be added to stable kernels, since, well, forever :) thanks, greg k-h
[PATCH v4] video: hyperv: hyperv_fb: Obtain screen resolution from Hyper-V host
Beginning from Windows 10 RS5+, VM screen resolution is obtained from host. The "video=hyperv_fb" boot time option is not needed, but still can be used to overwrite what the host specifies. The VM resolution on the host could be set by executing the powershell "set-vmvideo" command. Signed-off-by: Iouri Tarassov Signed-off-by: Wei Hu --- v2: - Implemented fallback when version negotiation failed. - Defined full size for supported_resolution array. v3: - Corrected the synthvid major and minor version comparison problem. v4: - Changed function name to synthvid_ver_ge(). drivers/video/fbdev/hyperv_fb.c | 159 +--- 1 file changed, 147 insertions(+), 12 deletions(-) diff --git a/drivers/video/fbdev/hyperv_fb.c b/drivers/video/fbdev/hyperv_fb.c index 00f5bdcc6c6f..fe319fc39bec 100644 --- a/drivers/video/fbdev/hyperv_fb.c +++ b/drivers/video/fbdev/hyperv_fb.c @@ -23,6 +23,14 @@ * * Portrait orientation is also supported: * For example: video=hyperv_fb:864x1152 + * + * When a Windows 10 RS5+ host is used, the virtual machine screen + * resolution is obtained from the host. The "video=hyperv_fb" option is + * not needed, but still can be used to overwrite what the host specifies. + * The VM resolution on the host could be set by executing the powershell + * "set-vmvideo" command. For example + * set-vmvideo -vmname name -horizontalresolution:1920 \ + * -verticalresolution:1200 -resolutiontype single */ #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt @@ -44,6 +52,10 @@ #define SYNTHVID_VERSION(major, minor) ((minor) << 16 | (major)) #define SYNTHVID_VERSION_WIN7 SYNTHVID_VERSION(3, 0) #define SYNTHVID_VERSION_WIN8 SYNTHVID_VERSION(3, 2) +#define SYNTHVID_VERSION_WIN10 SYNTHVID_VERSION(3, 5) + +#define SYNTHVID_VER_GET_MAJOR(ver) (ver & 0x) +#define SYNTHVID_VER_GET_MINOR(ver) ((ver & 0x) >> 16) #define SYNTHVID_DEPTH_WIN7 16 #define SYNTHVID_DEPTH_WIN8 32 @@ -82,16 +94,25 @@ enum synthvid_msg_type { SYNTHVID_POINTER_SHAPE = 8, SYNTHVID_FEATURE_CHANGE = 9, SYNTHVID_DIRT = 10, + SYNTHVID_RESOLUTION_REQUEST = 13, + SYNTHVID_RESOLUTION_RESPONSE= 14, - SYNTHVID_MAX= 11 + SYNTHVID_MAX= 15 }; +#defineSYNTHVID_EDID_BLOCK_SIZE128 +#defineSYNTHVID_MAX_RESOLUTION_COUNT 64 + +struct hvd_screen_info { + u16 width; + u16 height; +} __packed; + struct synthvid_msg_hdr { u32 type; u32 size; /* size of this header + payload after this field*/ } __packed; - struct synthvid_version_req { u32 version; } __packed; @@ -102,6 +123,19 @@ struct synthvid_version_resp { u8 max_video_outputs; } __packed; +struct synthvid_supported_resolution_req { + u8 maximum_resolution_count; +} __packed; + +struct synthvid_supported_resolution_resp { + u8 edid_block[SYNTHVID_EDID_BLOCK_SIZE]; + u8 resolution_count; + u8 default_resolution_index; + u8 is_standard; + struct hvd_screen_info + supported_resolution[SYNTHVID_MAX_RESOLUTION_COUNT]; +} __packed; + struct synthvid_vram_location { u64 user_ctx; u8 is_vram_gpa_specified; @@ -187,6 +221,8 @@ struct synthvid_msg { struct synthvid_pointer_shape ptr_shape; struct synthvid_feature_change feature_chg; struct synthvid_dirt dirt; + struct synthvid_supported_resolution_req resolution_req; + struct synthvid_supported_resolution_resp resolution_resp; }; } __packed; @@ -224,6 +260,8 @@ struct hvfb_par { static uint screen_width = HVFB_WIDTH; static uint screen_height = HVFB_HEIGHT; +static uint screen_width_max = HVFB_WIDTH; +static uint screen_height_max = HVFB_HEIGHT; static uint screen_depth; static uint screen_fb_size; @@ -354,6 +392,7 @@ static void synthvid_recv_sub(struct hv_device *hdev) /* Complete the wait event */ if (msg->vid_hdr.type == SYNTHVID_VERSION_RESPONSE || + msg->vid_hdr.type == SYNTHVID_RESOLUTION_RESPONSE || msg->vid_hdr.type == SYNTHVID_VRAM_LOCATION_ACK) { memcpy(par->init_buf, msg, MAX_VMBUS_PKT_SIZE); complete(&par->wait); @@ -400,6 +439,17 @@ static void synthvid_receive(void *ctx) } while (bytes_recvd > 0 && ret == 0); } +/* Check if the ver1 version is equal or greater than ver2 */ +static inline bool synthvid_ver_ge(u32 ver1, u32 ver2) +{ + if (SYNTHVID_VER_GET_MAJOR(ver1) > SYNTHVID_VER_GET_MAJOR(ver2) || + (SYNTHVID_VER_GET_MAJOR(ver1) == SYNTHVID_VER_GET_MAJOR(ver2) && +SYNTHVID_VER_GET_MINOR(ver1) >= SYNTHVID_VER_GET_MINOR(ver2))) + return true; + + return false; +} + /* Check synthetic video protocol version with the host */ static int synthvid_negotiate_ver(
Re: [PATCH] drm/nouveau: add missing single_release()
Reviewed-by: Karol Herbst On Thu, Sep 5, 2019 at 9:14 AM Wei Yongjun wrote: > > When using single_open() for opening, single_release() should be > used, otherwise there is a memory leak. > > This is detected by Coccinelle semantic patch. > > Fixes: 6e9fc177399f ("drm/nouveau/debugfs: add copy of sysfs pstate interface > ported to debugfs") > Signed-off-by: Wei Yongjun > --- > drivers/gpu/drm/nouveau/nouveau_debugfs.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/gpu/drm/nouveau/nouveau_debugfs.c > b/drivers/gpu/drm/nouveau/nouveau_debugfs.c > index 7dfbbbc1beea..35695f493271 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_debugfs.c > +++ b/drivers/gpu/drm/nouveau/nouveau_debugfs.c > @@ -202,6 +202,7 @@ static const struct file_operations nouveau_pstate_fops = > { > .open = nouveau_debugfs_pstate_open, > .read = seq_read, > .write = nouveau_debugfs_pstate_set, > + .release = single_release, > }; > > static struct drm_info_list nouveau_debugfs_list[] = { > > > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH AUTOSEL 4.19 147/167] drm/panel: Add support for Armadeus ST0700 Adapt
On Thu, Sep 5, 2019 at 10:03 AM Greg KH wrote: > > On Thu, Sep 05, 2019 at 09:55:58AM +0100, Rob Herring wrote: > > On Tue, Sep 3, 2019 at 5:31 PM Sasha Levin wrote: > > > > > > From: Sébastien Szymanski > > > > > > [ Upstream commit c479450f61c7f1f248c9a54aedacd2a6ca521ff8 ] > > > > > > This patch adds support for the Armadeus ST0700 Adapt. It comes with a > > > Santek ST0700I5Y-RBSLW 7.0" WVGA (800x480) TFT and an adapter board so > > > that it can be connected on the TFT header of Armadeus Dev boards. > > > > > > Cc: sta...@vger.kernel.org # v4.19 > > > Reviewed-by: Rob Herring > > > Signed-off-by: Sébastien Szymanski > > > Signed-off-by: Sam Ravnborg > > > Link: > > > https://patchwork.freedesktop.org/patch/msgid/20190507152713.27494-1-sebastien.szyman...@armadeus.com > > > Signed-off-by: Sasha Levin > > > --- > > > .../display/panel/armadeus,st0700-adapt.txt | 9 ++ > > > drivers/gpu/drm/panel/panel-simple.c | 29 +++ > > > 2 files changed, 38 insertions(+) > > > create mode 100644 > > > Documentation/devicetree/bindings/display/panel/armadeus,st0700-adapt.txt > > > > Looks like a new feature, not stable material. Not sure why it got > > tagged for stable. > > New device ids/tables are able to be added to stable kernels, since, > well, forever :) Yes I know, but I wouldn't put new panels in that category though I guess it's just data. If we are, then you should be picking up just about every single commit to panel-simple.c for stable. Rob
Re: [PATCH 5/7] drm/omap: Enable COLOR_ENCODING and COLOR_RANGE properties for planes
On 03/09/2019 18:32, Laurent Pinchart wrote: > Hi Tomi, > > Thank you for the patch. > > On Mon, Sep 02, 2019 at 03:53:57PM +0300, Tomi Valkeinen wrote: >> From: Jyri Sarha >> >> Adds support for COLOR_ENCODING and COLOR_RANGE properties to >> omap_plane.c and dispc.c. The supported encodings and ranges are: >> >> For COLOR_ENCODING: >> - YCbCt BT.601 (default) > > Did you mean YCbCr ? > >> - YCbCt BT.709 >> >> For COLOR_RANGE: >> - YCbCt limited range >> - YCbCt full range (default) >> >> Signed-off-by: Jyri Sarha >> Signed-off-by: Tomi Valkeinen >> --- >> drivers/gpu/drm/omapdrm/dss/dispc.c | 119 -- >> drivers/gpu/drm/omapdrm/dss/omapdss.h | 4 + >> drivers/gpu/drm/omapdrm/omap_plane.c | 30 +++ >> 3 files changed, 127 insertions(+), 26 deletions(-) >> >> diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c >> b/drivers/gpu/drm/omapdrm/dss/dispc.c >> index 7d87d60edb80..40ddb28ee7aa 100644 >> --- a/drivers/gpu/drm/omapdrm/dss/dispc.c >> +++ b/drivers/gpu/drm/omapdrm/dss/dispc.c >> @@ -892,32 +892,91 @@ static void dispc_wb_write_color_conv_coef(struct >> dispc_device *dispc, >> #undef CVAL >> } >> >> -static void dispc_setup_color_conv_coef(struct dispc_device *dispc) >> +/* YUV -> RGB, ITU-R BT.601, full range */ >> +const static struct csc_coef_yuv2rgb coefs_yuv2rgb_bt601_full = { > > static const is usually preferred over const static, here and below. > >> +256, 0, 358, /* ry, rcb, rcr |1.000 0.000 1.402|*/ >> +256, -88, -182, /* gy, gcb, gcr |1.000 -0.344 -0.714|*/ >> +256, 452,0, /* by, bcb, bcr |1.000 1.772 0.000|*/ >> +true, /* full range */ >> +}; >> + >> +/* YUV -> RGB, ITU-R BT.601, limited range */ >> +const static struct csc_coef_yuv2rgb coefs_yuv2rgb_bt601_lim = { >> +298,0, 409,/* ry, rcb, rcr |1.164 0.000 1.596|*/ >> +298, -100, -208,/* gy, gcb, gcr |1.164 -0.392 -0.813|*/ >> +298, 516,0,/* by, bcb, bcr |1.164 2.017 0.000|*/ >> +false, /* limited range */ >> +}; >> + >> +/* YUV -> RGB, ITU-R BT.709, full range */ >> +const static struct csc_coef_yuv2rgb coefs_yuv2rgb_bt709_full = { >> +256,0, 402,/* ry, rcb, rcr |1.000 0.000 1.570|*/ >> +256, -48, -120,/* gy, gcb, gcr |1.000 -0.187 -0.467|*/ >> +256, 475,0,/* by, bcb, bcr |1.000 1.856 0.000|*/ >> +true, /* full range */ >> +}; >> + >> +/* YUV -> RGB, ITU-R BT.709, limited range */ >> +const static struct csc_coef_yuv2rgb coefs_yuv2rgb_bt709_lim = { >> +298,0, 459,/* ry, rcb, rcr |1.164 0.000 1.793|*/ >> +298, -55, -136,/* gy, gcb, gcr |1.164 -0.213 -0.533|*/ >> +298, 541,0,/* by, bcb, bcr |1.164 2.112 0.000|*/ >> +false, /* limited range */ >> +}; >> + >> +/* RGB -> YUV, ITU-R BT.601, limited range */ >> +const static struct csc_coef_rgb2yuv coefs_rgb2yuv_bt601_lim = { >> + 66, 129, 25, /* yr, yg, yb | 0.257 0.504 0.098|*/ >> +-38, -74, 112, /* cbr, cbg, cbb |-0.148 -0.291 0.439|*/ >> +112, -94, -18, /* crr, crg, crb | 0.439 -0.368 -0.071|*/ >> +false, /* limited range */ >> +}; >> + >> +/* RGB -> YUV, ITU-R BT.601, full range */ >> +const static struct csc_coef_rgb2yuv coefs_rgb2yuv_bt601_full = { >> + 77, 150, 29, /* yr, yg, yb | 0.299 0.587 0.114|*/ >> +-43, -85, 128, /* cbr, cbg, cbb |-0.173 -0.339 0.511|*/ >> +128, -107, -21, /* crr, crg, crb | 0.511 -0.428 -0.083|*/ >> +true, /* full range */ >> +}; >> + >> +/* RGB -> YUV, ITU-R BT.709, limited range */ >> +const static struct csc_coef_rgb2yuv coefs_rgb2yuv_bt701_lim = { > > That should be coefs_rgb2yuv_bt709_lim > >> + 47, 157, 16,/* yr, yg, yb | 0.1826 0.6142 0.0620|*/ >> +-26, -87, 112,/* cbr, cbg, cbb |-0.1006 -0.3386 0.4392|*/ >> +112, -102, -10,/* crr, crg, crb | 0.4392 -0.3989 -0.0403|*/ >> +false, /* limited range */ >> +}; > > Why is coefs_rgb2yuv_bt709_full not supported ? Actually > coefs_rgb2yuv_bt601_lim and coefs_rgb2yuv_bt701_lim seem unused. > I must have simply forgotten. This is an old patch and I can not remember exactly. But I remember that I wanted to add all CSCs at one time so that I do not need start from the beginning when ever a new conversion is asked. For the moment rgb to yuv conversions are only used for write back and it currently only uses the default BT.601 Full range. I'll add rgb2yuv_bt709_full. Or should I remove all the unused CSCs? Or maybe put them behind some ifdef so that they do not bloat the kernel? >> + >> +static int dispc_ovl_set_csc(struct dispc_device *dispc, >> + enum omap_plane_id plane, >> + enum drm_color_encoding color_encoding, >> +
Re: [PATCH v2 0/3] ast, mgag200: Map console BO while it's being displayed
On Thu, Sep 05, 2019 at 10:19:40AM +0200, Thomas Zimmermann wrote: > Hi > > Am 05.09.19 um 09:56 schrieb Daniel Vetter: > > On Thu, Sep 5, 2019 at 9:01 AM Gerd Hoffmann wrote: > >> > >> Hi, > >> > >>> - imo we should fix this by using the io_mapping stuff, that avoids > >>> the overhead of repeated pat checks for map/unmap. > >> > >> Another idea: IIRC ttm has a move_notify callback. So we could simply > >> keep mappings active even when the refcount goes down to zero. Then do > >> the actual unmap either in the move_notify or in the destroy callback. > > > > Yeah that should be a really clean solution, and only needs changes in > > the vram helpers. Which is nice, more common code! > > But the console's BO is special wrt to mapping. Putting special code for > console handling into vram helpers doesn't seem right. I have no special handling in mind. I think we can simply do that for all gem objects, no matter whenever they are created by fbcon or userspace (wayland/xorg/whatever). vmap will create a mapping (or increase the reference count). vunmap decreases the reference count, when it goes down to zero unpin it but keep the mapping. The actual unmap happens when ttm wants move the object from VRAM to SYSTEM due to VRAM being full. In case vram has room for all our objects we simply never unmap. hope this clarifies, Gerd ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 7/8] drm/vmwgfx: switch to own vma manager
On Thu, Sep 05, 2019 at 08:31:34AM +, Thomas Hellstrom wrote: > On Thu, 2019-09-05 at 09:05 +0200, Gerd Hoffmann wrote: > > Add struct drm_vma_offset_manager to vma_private, initialize it and > > pass it to ttm_bo_device_init(). > > > > With this in place the last user of ttm's embedded vma offset manager > > is gone and we can remove it (in a separate patch). > > > > Signed-off-by: Gerd Hoffmann > > --- > > drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 1 + > > drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 6 +- > > 2 files changed, 6 insertions(+), 1 deletion(-) > > > > > > Reviewed-by: Thomas Hellström > > I assume this will be merged through drm-misc? Yes, that is the plan (after collecting acks for all drivers). cheers, Gerd
Re: [PATCH] drm/panfrost: Fix regulator_get_optional() misuse
On 05/09/2019 09:21, Rob Herring wrote: > +Steven > > On Wed, Sep 4, 2019 at 1:30 PM Mark Brown wrote: >> >> The panfrost driver requests a supply using regulator_get_optional() >> but both the name of the supply and the usage pattern suggest that it is >> being used for the main power for the device and is not at all optional >> for the device for function, there is no meaningful handling for absent >> supplies. Such regulators should use the vanilla regulator_get() >> interface, it will ensure that even if a supply is not described in the >> system integration one will be provided in software. > > I guess commits e21dd290881b ("drm/panfrost: Enable devfreq to work > without regulator") and c90f30812a79 ("drm/panfrost: Add missing check > for pfdev->regulator") > in -next should be reverted or partially reverted? Ah, I didn't realise that regulator_get() will return a dummy regulator if none is provided in the DT. In theory that seems like a nicer solution to my two commits. However there's still a problem - the dummy regulator returned from regulator_get() reports errors when regulator_set_voltage() is called. So I get errors like this: [ 299.861165] panfrost e82c.mali: Cannot set voltage 110 uV [ 299.867294] devfreq devfreq0: dvfs failed with (-22) error (And therefore the frequency isn't being changed) Ideally we want a dummy regulator that will silently ignore any regulator_set_voltage() calls. Steve
[PATCH v2] drm: bridge/dw_hdmi: add audio sample channel status setting
From: Yakir Yang When transmitting IEC60985 linear PCM audio, we configure the Aduio Sample Channel Status information of all the channel status bits in the IEC60958 frame. Refer to 60958-3 page 10 for frequency, original frequency, and wordlength setting. This fix the issue that audio does not come out on some monitors (e.g. LG 22CV241) Note that these registers are only for interfaces: I2S audio interface, General Purpose Audio (GPA), or AHB audio DMA (AHBAUDDMA). For S/PDIF interface this information comes from the stream. Currently this function dw_hdmi_set_channel_status is only called from dw-hdmi-i2s-audio in I2S setup. Signed-off-by: Yakir Yang Signed-off-by: Cheng-Yi Chiang --- Original patch by Yakir Yang is at https://lore.kernel.org/patchwork/patch/539653/ Change from v1 to v2: 1. Remove the version check because this will only be called by dw-hdmi-i2s-audio, and the registers are available in I2S setup. 2. Set these registers in dw_hdmi_i2s_hw_params. 3. Fix the sample width setting so it can use 16 or 24 bits. .../drm/bridge/synopsys/dw-hdmi-i2s-audio.c | 1 + drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 70 +++ drivers/gpu/drm/bridge/synopsys/dw-hdmi.h | 20 ++ include/drm/bridge/dw_hdmi.h | 2 + 4 files changed, 93 insertions(+) diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c index 34d8e837555f..b801a28b0f17 100644 --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c @@ -102,6 +102,7 @@ static int dw_hdmi_i2s_hw_params(struct device *dev, void *data, } dw_hdmi_set_sample_rate(hdmi, hparms->sample_rate); + dw_hdmi_set_channel_status(hdmi, hparms->sample_width); dw_hdmi_set_channel_count(hdmi, hparms->channels); dw_hdmi_set_channel_allocation(hdmi, hparms->cea.channel_allocation); diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index bd65d0479683..d1daa369c8ae 100644 --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c @@ -582,6 +582,76 @@ static unsigned int hdmi_compute_n(unsigned int freq, unsigned long pixel_clk) return n; } +/* + * When transmitting IEC60958 linear PCM audio, these registers allow to + * configure the channel status information of all the channel status + * bits in the IEC60958 frame. For the moment this configuration is only + * used when the I2S audio interface, General Purpose Audio (GPA), + * or AHB audio DMA (AHBAUDDMA) interface is active + * (for S/PDIF interface this information comes from the stream). + */ +void dw_hdmi_set_channel_status(struct dw_hdmi *hdmi, + unsigned int sample_width) +{ + u8 aud_schnl_samplerate; + u8 aud_schnl_8; + u8 word_length_bits; + + switch (hdmi->sample_rate) { + case 32000: + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_32K; + break; + case 44100: + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_44K1; + break; + case 48000: + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_48K; + break; + case 88200: + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_88K2; + break; + case 96000: + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_96K; + break; + case 176400: + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_176K4; + break; + case 192000: + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_192K; + break; + case 768000: + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_768K; + break; + default: + dev_warn(hdmi->dev, "Unsupported audio sample rate (%u)\n", +hdmi->sample_rate); + return; + } + + /* set channel status register */ + hdmi_modb(hdmi, aud_schnl_samplerate, HDMI_FC_AUDSCHNLS7_SMPRATE_MASK, + HDMI_FC_AUDSCHNLS7); + + /* +* Set original frequency to be the same as frequency. +* Use one-complement value as stated in IEC60958-3 page 13. +*/ + aud_schnl_8 = (~aud_schnl_samplerate) << + HDMI_FC_AUDSCHNLS8_ORIGSAMPFREQ_OFFSET; + + /* +* Refer to IEC60958-3 page 12. We can accept 16 bits or 24 bits. +* Otherwise, set the register to 0t o indicate using default value. +*/ + word_length_bits = (sample_width == 16) ? 0x2 : + ((sample_width == 24) ? 0xb : 0); + + aud_schnl_8 |= word_length_bits << HDMI_FC_AUDSCHNLS8_WORDLEGNTH_OFFSET; + + hdmi_writeb(hdmi, aud_schnl_8, HDMI_FC_AUDSCHNLS8); +} +EXPORT_SYMBOL_GPL(dw_hdmi_set_channel_st
Re: [PATCH 5/7] drm/omap: Enable COLOR_ENCODING and COLOR_RANGE properties for planes
Hi Jyri, On Thu, Sep 05, 2019 at 12:24:37PM +0300, Jyri Sarha wrote: > On 03/09/2019 18:32, Laurent Pinchart wrote: > > On Mon, Sep 02, 2019 at 03:53:57PM +0300, Tomi Valkeinen wrote: > >> From: Jyri Sarha > >> > >> Adds support for COLOR_ENCODING and COLOR_RANGE properties to > >> omap_plane.c and dispc.c. The supported encodings and ranges are: > >> > >> For COLOR_ENCODING: > >> - YCbCt BT.601 (default) > > > > Did you mean YCbCr ? > > >> - YCbCt BT.709 > >> > >> For COLOR_RANGE: > >> - YCbCt limited range > >> - YCbCt full range (default) > >> > >> Signed-off-by: Jyri Sarha > >> Signed-off-by: Tomi Valkeinen > >> --- > >> drivers/gpu/drm/omapdrm/dss/dispc.c | 119 -- > >> drivers/gpu/drm/omapdrm/dss/omapdss.h | 4 + > >> drivers/gpu/drm/omapdrm/omap_plane.c | 30 +++ > >> 3 files changed, 127 insertions(+), 26 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c > >> b/drivers/gpu/drm/omapdrm/dss/dispc.c > >> index 7d87d60edb80..40ddb28ee7aa 100644 > >> --- a/drivers/gpu/drm/omapdrm/dss/dispc.c > >> +++ b/drivers/gpu/drm/omapdrm/dss/dispc.c > >> @@ -892,32 +892,91 @@ static void dispc_wb_write_color_conv_coef(struct > >> dispc_device *dispc, > >> #undef CVAL > >> } > >> > >> -static void dispc_setup_color_conv_coef(struct dispc_device *dispc) > >> +/* YUV -> RGB, ITU-R BT.601, full range */ > >> +const static struct csc_coef_yuv2rgb coefs_yuv2rgb_bt601_full = { > > > > static const is usually preferred over const static, here and below. > > > >> + 256, 0, 358, /* ry, rcb, rcr |1.000 0.000 1.402|*/ > >> + 256, -88, -182, /* gy, gcb, gcr |1.000 -0.344 -0.714|*/ > >> + 256, 452,0, /* by, bcb, bcr |1.000 1.772 0.000|*/ > >> + true, /* full range */ > >> +}; > >> + > >> +/* YUV -> RGB, ITU-R BT.601, limited range */ > >> +const static struct csc_coef_yuv2rgb coefs_yuv2rgb_bt601_lim = { > >> + 298,0, 409,/* ry, rcb, rcr |1.164 0.000 1.596|*/ > >> + 298, -100, -208,/* gy, gcb, gcr |1.164 -0.392 -0.813|*/ > >> + 298, 516,0,/* by, bcb, bcr |1.164 2.017 0.000|*/ > >> + false, /* limited range */ > >> +}; > >> + > >> +/* YUV -> RGB, ITU-R BT.709, full range */ > >> +const static struct csc_coef_yuv2rgb coefs_yuv2rgb_bt709_full = { > >> + 256,0, 402,/* ry, rcb, rcr |1.000 0.000 1.570|*/ > >> + 256, -48, -120,/* gy, gcb, gcr |1.000 -0.187 -0.467|*/ > >> + 256, 475,0,/* by, bcb, bcr |1.000 1.856 0.000|*/ > >> + true, /* full range */ > >> +}; > >> + > >> +/* YUV -> RGB, ITU-R BT.709, limited range */ > >> +const static struct csc_coef_yuv2rgb coefs_yuv2rgb_bt709_lim = { > >> + 298,0, 459,/* ry, rcb, rcr |1.164 0.000 1.793|*/ > >> + 298, -55, -136,/* gy, gcb, gcr |1.164 -0.213 -0.533|*/ > >> + 298, 541,0,/* by, bcb, bcr |1.164 2.112 0.000|*/ > >> + false, /* limited range */ > >> +}; > >> + > >> +/* RGB -> YUV, ITU-R BT.601, limited range */ > >> +const static struct csc_coef_rgb2yuv coefs_rgb2yuv_bt601_lim = { > >> + 66, 129, 25, /* yr, yg, yb | 0.257 0.504 0.098|*/ > >> + -38, -74, 112, /* cbr, cbg, cbb |-0.148 -0.291 0.439|*/ > >> + 112, -94, -18, /* crr, crg, crb | 0.439 -0.368 -0.071|*/ > >> + false, /* limited range */ > >> +}; > >> + > >> +/* RGB -> YUV, ITU-R BT.601, full range */ > >> +const static struct csc_coef_rgb2yuv coefs_rgb2yuv_bt601_full = { > >> + 77, 150, 29, /* yr, yg, yb | 0.299 0.587 0.114|*/ > >> + -43, -85, 128, /* cbr, cbg, cbb |-0.173 -0.339 0.511|*/ > >> + 128, -107, -21, /* crr, crg, crb | 0.511 -0.428 -0.083|*/ > >> + true, /* full range */ > >> +}; > >> + > >> +/* RGB -> YUV, ITU-R BT.709, limited range */ > >> +const static struct csc_coef_rgb2yuv coefs_rgb2yuv_bt701_lim = { > > > > That should be coefs_rgb2yuv_bt709_lim > > > >> + 47, 157, 16,/* yr, yg, yb | 0.1826 0.6142 0.0620|*/ > >> + -26, -87, 112,/* cbr, cbg, cbb |-0.1006 -0.3386 0.4392|*/ > >> + 112, -102, -10,/* crr, crg, crb | 0.4392 -0.3989 -0.0403|*/ > >> + false, /* limited range */ > >> +}; > > > > Why is coefs_rgb2yuv_bt709_full not supported ? Actually > > coefs_rgb2yuv_bt601_lim and coefs_rgb2yuv_bt701_lim seem unused. > > > > I must have simply forgotten. This is an old patch and I can not > remember exactly. But I remember that I wanted to add all CSCs at one > time so that I do not need start from the beginning when ever a new > conversion is asked. For the moment rgb to yuv conversions are only used > for write back and it currently only uses the default BT.601 Full range. > > I'll add rgb2yuv_bt709_full. Or should I remove all the unused CSCs? Or > maybe put them behind some ifdef so that they do not bloat the kernel? Commented-out code is fro
Re: [PATCH v2] drm: bridge/dw_hdmi: add audio sample channel status setting
Sorry for the noise. Removed original author y...@rock-chips.com from the thread because that mail is obsolete. Yakir is now using kuankua...@gmail.com. On Thu, Sep 5, 2019 at 5:43 PM Cheng-Yi Chiang wrote: > > From: Yakir Yang > > When transmitting IEC60985 linear PCM audio, we configure the > Aduio Sample Channel Status information of all the channel > status bits in the IEC60958 frame. > Refer to 60958-3 page 10 for frequency, original frequency, and > wordlength setting. > > This fix the issue that audio does not come out on some monitors > (e.g. LG 22CV241) > > Note that these registers are only for interfaces: > I2S audio interface, General Purpose Audio (GPA), or AHB audio DMA > (AHBAUDDMA). > For S/PDIF interface this information comes from the stream. > > Currently this function dw_hdmi_set_channel_status is only called > from dw-hdmi-i2s-audio in I2S setup. > > Signed-off-by: Yakir Yang > Signed-off-by: Cheng-Yi Chiang > --- > Original patch by Yakir Yang is at > > https://lore.kernel.org/patchwork/patch/539653/ > > Change from v1 to v2: > 1. Remove the version check because this will only be called by > dw-hdmi-i2s-audio, and the registers are available in I2S setup. > 2. Set these registers in dw_hdmi_i2s_hw_params. > 3. Fix the sample width setting so it can use 16 or 24 bits. > > .../drm/bridge/synopsys/dw-hdmi-i2s-audio.c | 1 + > drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 70 +++ > drivers/gpu/drm/bridge/synopsys/dw-hdmi.h | 20 ++ > include/drm/bridge/dw_hdmi.h | 2 + > 4 files changed, 93 insertions(+) > > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c > b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c > index 34d8e837555f..b801a28b0f17 100644 > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c > @@ -102,6 +102,7 @@ static int dw_hdmi_i2s_hw_params(struct device *dev, void > *data, > } > > dw_hdmi_set_sample_rate(hdmi, hparms->sample_rate); > + dw_hdmi_set_channel_status(hdmi, hparms->sample_width); > dw_hdmi_set_channel_count(hdmi, hparms->channels); > dw_hdmi_set_channel_allocation(hdmi, hparms->cea.channel_allocation); > > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > index bd65d0479683..d1daa369c8ae 100644 > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > @@ -582,6 +582,76 @@ static unsigned int hdmi_compute_n(unsigned int freq, > unsigned long pixel_clk) > return n; > } > > +/* > + * When transmitting IEC60958 linear PCM audio, these registers allow to > + * configure the channel status information of all the channel status > + * bits in the IEC60958 frame. For the moment this configuration is only > + * used when the I2S audio interface, General Purpose Audio (GPA), > + * or AHB audio DMA (AHBAUDDMA) interface is active > + * (for S/PDIF interface this information comes from the stream). > + */ > +void dw_hdmi_set_channel_status(struct dw_hdmi *hdmi, > + unsigned int sample_width) > +{ > + u8 aud_schnl_samplerate; > + u8 aud_schnl_8; > + u8 word_length_bits; > + > + switch (hdmi->sample_rate) { > + case 32000: > + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_32K; > + break; > + case 44100: > + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_44K1; > + break; > + case 48000: > + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_48K; > + break; > + case 88200: > + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_88K2; > + break; > + case 96000: > + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_96K; > + break; > + case 176400: > + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_176K4; > + break; > + case 192000: > + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_192K; > + break; > + case 768000: > + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_768K; > + break; > + default: > + dev_warn(hdmi->dev, "Unsupported audio sample rate (%u)\n", > +hdmi->sample_rate); > + return; > + } > + > + /* set channel status register */ > + hdmi_modb(hdmi, aud_schnl_samplerate, HDMI_FC_AUDSCHNLS7_SMPRATE_MASK, > + HDMI_FC_AUDSCHNLS7); > + > + /* > +* Set original frequency to be the same as frequency. > +* Use one-complement value as stated in IEC60958-3 page 13. > +*/ > + aud_schnl_8 = (~aud_schnl_samplerate) << > + HDMI_FC_AUDSCHNLS8_ORIGSAMPFREQ_OFFSET; > + > + /* > +* Refer to IEC60958-3
Re: [PATCH v3 08/14] drm: rcar-du: Add support for CMM
Hi Laurent, On Tue, Aug 27, 2019 at 07:34:23PM +0300, Laurent Pinchart wrote: > Hi Laurent, > > On Tue, Aug 27, 2019 at 04:56:19PM +0200, Jacopo Mondi wrote: > > On Tue, Aug 27, 2019 at 03:24:22AM +0300, Laurent Pinchart wrote: > > > On Sun, Aug 25, 2019 at 03:51:48PM +0200, Jacopo Mondi wrote: > > > > Add a driver for the R-Car Display Unit Color Correction Module. > > > > > > > > In most of Gen3 SoCs, each DU output channel is provided with a CMM unit > > > > to perform image enhancement and color correction. > > > > > > > > Add support for CMM through a driver that supports configuration of > > > > the 1-dimensional LUT table. More advanced CMM feature will be > > > > implemented on top of this basic one. > > > > > > > > Signed-off-by: Jacopo Mondi > > > > --- > > > > drivers/gpu/drm/rcar-du/Kconfig| 7 + > > > > drivers/gpu/drm/rcar-du/Makefile | 1 + > > > > drivers/gpu/drm/rcar-du/rcar_cmm.c | 262 + > > > > drivers/gpu/drm/rcar-du/rcar_cmm.h | 38 + > > > > 4 files changed, 308 insertions(+) > > > > create mode 100644 drivers/gpu/drm/rcar-du/rcar_cmm.c > > > > create mode 100644 drivers/gpu/drm/rcar-du/rcar_cmm.h > > > > > > > > diff --git a/drivers/gpu/drm/rcar-du/Kconfig > > > > b/drivers/gpu/drm/rcar-du/Kconfig > > > > index 1529849e217e..539d232790d1 100644 > > > > --- a/drivers/gpu/drm/rcar-du/Kconfig > > > > +++ b/drivers/gpu/drm/rcar-du/Kconfig > > > > @@ -13,6 +13,13 @@ config DRM_RCAR_DU > > > > Choose this option if you have an R-Car chipset. > > > > If M is selected the module will be called rcar-du-drm. > > > > > > > > +config DRM_RCAR_CMM > > > > + bool "R-Car DU Color Management Module (CMM) Support" > > > > + depends on DRM && OF > > > > + depends on DRM_RCAR_DU > > > > + help > > > > + Enable support for R-Car Color Management Module (CMM). > > > > + > > > > config DRM_RCAR_DW_HDMI > > > > tristate "R-Car DU Gen3 HDMI Encoder Support" > > > > depends on DRM && OF > > > > diff --git a/drivers/gpu/drm/rcar-du/Makefile > > > > b/drivers/gpu/drm/rcar-du/Makefile > > > > index 6c2ed9c46467..4d1187ccc3e5 100644 > > > > --- a/drivers/gpu/drm/rcar-du/Makefile > > > > +++ b/drivers/gpu/drm/rcar-du/Makefile > > > > @@ -15,6 +15,7 @@ rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS) += rcar_du_of.o > > > > \ > > > > rcar-du-drm-$(CONFIG_DRM_RCAR_VSP) += rcar_du_vsp.o > > > > rcar-du-drm-$(CONFIG_DRM_RCAR_WRITEBACK) += rcar_du_writeback.o > > > > > > > > +obj-$(CONFIG_DRM_RCAR_CMM) += rcar_cmm.o > > > > obj-$(CONFIG_DRM_RCAR_DU) += rcar-du-drm.o > > > > obj-$(CONFIG_DRM_RCAR_DW_HDMI) += rcar_dw_hdmi.o > > > > obj-$(CONFIG_DRM_RCAR_LVDS)+= rcar_lvds.o > > > > diff --git a/drivers/gpu/drm/rcar-du/rcar_cmm.c > > > > b/drivers/gpu/drm/rcar-du/rcar_cmm.c > > > > new file mode 100644 > > > > index ..55361f5701e8 > > > > --- /dev/null > > > > +++ b/drivers/gpu/drm/rcar-du/rcar_cmm.c > > > > @@ -0,0 +1,262 @@ > > > > +// SPDX-License-Identifier: GPL-2.0+ > > > > +/* > > > > + * rcar_cmm.c -- R-Car Display Unit Color Management Module > > > > + * > > > > + * Copyright (C) 2019 Jacopo Mondi > > > > + */ > > > > + > > > > +#include > > > > +#include > > > > +#include > > > > +#include > > > > +#include > > > > + > > > > +#include > > > > + > > > > +#include "rcar_cmm.h" > > > > + > > > > +#define CM2_LUT_CTRL 0x > > > > +#define CM2_LUT_CTRL_LUT_ENBIT(0) > > > > +#define CM2_LUT_TBL_BASE 0x0600 > > > > +#define CM2_LUT_TBL(__i) (CM2_LUT_TBL_BASE + (__i) * 4) > > > > + > > > > +struct rcar_cmm { > > > > + void __iomem *base; > > > > + bool enabled; > > > > + > > > > + /* > > > > +* @lut:1D-LUT status > > > > +* @lut.enabled:1D-LUT enabled flag > > > > +* @lut.size: Number of entries in the LUT table > > > > > > Please see my review of patch 13/14, I wonder if we could drop this > > > field. > > > > > > > +* @lut.table: Table of 1D-LUT entries scaled to HW > > > > support > > > > +* precision (8-bits per color component) > > > > +*/ > > > > + struct { > > > > + bool enabled; > > > > + unsigned int size; > > > > + u32 table[CMM_GAMMA_LUT_SIZE]; > > > > + } lut; > > > > +}; > > > > + > > > > +static inline int rcar_cmm_read(struct rcar_cmm *rcmm, u32 reg) > > > > +{ > > > > + return ioread32(rcmm->base + reg); > > > > +} > > > > + > > > > +static inline void rcar_cmm_write(struct rcar_cmm *rcmm, u32 reg, u32 > > > > data) > > > > +{ > > > > + iowrite32(data, rcmm->base + reg); > > > > +} > > > > + > > > > +/* > > > > + * rcar_cmm_lut_extract() - Scale down to hw precision the DRM LUT > > > > table > > > > > > s/hw/hardware/ (and below too) > > > > > > > + * entries and store them. >
Re: [PATCH 4/7] drm/omap: Implement CTM property for CRTC using OVL managers CPR matrix
On 05/09/2019 00:52, Laurent Pinchart wrote: >> static void omap_crtc_write_crtc_properties(struct drm_crtc *crtc) >> { >> struct omap_drm_private *priv = crtc->dev->dev_private; >> @@ -402,7 +428,16 @@ static void omap_crtc_write_crtc_properties(struct >> drm_crtc *crtc) >> info.default_color = 0x00; >> info.trans_enabled = false; >> info.partial_alpha_enabled = false; >> -info.cpr_enable = false; >> + >> +if (crtc->state->ctm) { >> +struct drm_color_ctm *ctm = >> +(struct drm_color_ctm *) crtc->state->ctm->data; >> + >> +info.cpr_enable = true; >> +omap_crtc_cpr_coefs_from_ctm(ctm, &info.cpr_coefs); > As an optimisation it would be nice to only write the coefficients when > they actually change. That could be implemented on top of this series. E.g. apply this ? - if (crtc->state->ctm) + if (crtc->state->color_mgmt_changed && crtc->state->ctm) >>> Something like that, but .mgr_setup() should then be taught not to write >>> unchanged CTM tables to registers. Do you think it would be worth it ? >> Hmmm, jess I should do it like this: >> if (crtc->state->color_mgmt_changed) { >> if (crtc->state->ctm) { >> ... >> +} else { >> +info.cpr_enable = false; >> +} >> } >> >> This way the whole CPR functionality is turned off, if the there is no >> CTM in the crtc state. > Yes, but you would also need to update .mgr_setup() :-) A new > color_mgmt_changed flag would be needed in the info structure too. > I am starting to thing that such an "optimization" may not be worth the added complexity. The arithmetic and writing three registers is not that costly and we do not commit a new crtc state that often. If we later consider otherwise we can add the optimization as a separate patch. BR, Jyri -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/7] drm/omap: Implement CTM property for CRTC using OVL managers CPR matrix
Hi Jyri, On Thu, Sep 05, 2019 at 01:00:51PM +0300, Jyri Sarha wrote: > On 05/09/2019 00:52, Laurent Pinchart wrote: > >> static void omap_crtc_write_crtc_properties(struct drm_crtc *crtc) > >> { > >>struct omap_drm_private *priv = crtc->dev->dev_private; > >> @@ -402,7 +428,16 @@ static void > >> omap_crtc_write_crtc_properties(struct drm_crtc *crtc) > >>info.default_color = 0x00; > >>info.trans_enabled = false; > >>info.partial_alpha_enabled = false; > >> - info.cpr_enable = false; > >> + > >> + if (crtc->state->ctm) { > >> + struct drm_color_ctm *ctm = > >> + (struct drm_color_ctm *) crtc->state->ctm->data; > >> + > >> + info.cpr_enable = true; > >> + omap_crtc_cpr_coefs_from_ctm(ctm, &info.cpr_coefs); > > > > As an optimisation it would be nice to only write the coefficients when > > they actually change. That could be implemented on top of this series. > > E.g. apply this ? > > - if (crtc->state->ctm) > + if (crtc->state->color_mgmt_changed && crtc->state->ctm) > >>> > >>> Something like that, but .mgr_setup() should then be taught not to write > >>> unchanged CTM tables to registers. Do you think it would be worth it ? > >> > >> Hmmm, jess I should do it like this: > >> if (crtc->state->color_mgmt_changed) { > >>if (crtc->state->ctm) { > >> ... > >> + } else { > >> + info.cpr_enable = false; > >> + } > >> } > >> > >> This way the whole CPR functionality is turned off, if the there is no > >> CTM in the crtc state. > > > > Yes, but you would also need to update .mgr_setup() :-) A new > > color_mgmt_changed flag would be needed in the info structure too. > > I am starting to thing that such an "optimization" may not be worth the > added complexity. The arithmetic and writing three registers is not that > costly and we do not commit a new crtc state that often. We call omap_crtc_write_crtc_properties() in omap_crtc_atomic_flush(), so that's at every page flip... > If we later consider otherwise we can add the optimization as a separate > patch. -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [LKP] [drm/mgag200] 90f479ae51: vm-scalability.median -18.8% regression
On Thu, Sep 5, 2019 at 8:58 AM Feng Tang wrote: > > Hi Vetter, > > On Wed, Sep 04, 2019 at 01:20:29PM +0200, Daniel Vetter wrote: > > On Wed, Sep 4, 2019 at 1:15 PM Dave Airlie wrote: > > > > > > On Wed, 4 Sep 2019 at 19:17, Daniel Vetter wrote: > > > > > > > > On Wed, Sep 4, 2019 at 10:35 AM Feng Tang wrote: > > > > > > > > > > Hi Daniel, > > > > > > > > > > On Wed, Sep 04, 2019 at 10:11:11AM +0200, Daniel Vetter wrote: > > > > > > On Wed, Sep 4, 2019 at 8:53 AM Thomas Zimmermann > > > > > > wrote: > > > > > > > > > > > > > > Hi > > > > > > > > > > > > > > Am 04.09.19 um 08:27 schrieb Feng Tang: > > > > > > > >> Thank you for testing. But don't get too excited, because the > > > > > > > >> patch > > > > > > > >> simulates a bug that was present in the original mgag200 code. > > > > > > > >> A > > > > > > > >> significant number of frames are simply skipped. That is > > > > > > > >> apparently the > > > > > > > >> reason why it's faster. > > > > > > > > > > > > > > > > Thanks for the detailed info, so the original code skips > > > > > > > > time-consuming > > > > > > > > work inside atomic context on purpose. Is there any space to > > > > > > > > optmise it? > > > > > > > > If 2 scheduled update worker are handled at almost same time, > > > > > > > > can one be > > > > > > > > skipped? > > > > > > > > > > > > > > To my knowledge, there's only one instance of the worker. > > > > > > > Re-scheduling > > > > > > > the worker before a previous instance started, will not create a > > > > > > > second > > > > > > > instance. The worker's instance will complete all pending > > > > > > > updates. So in > > > > > > > some way, skipping workers already happens. > > > > > > > > > > > > So I think that the most often fbcon update from atomic context is > > > > > > the > > > > > > blinking cursor. If you disable that one you should be back to the > > > > > > old > > > > > > performance level I think, since just writing to dmesg is from > > > > > > process > > > > > > context, so shouldn't change. > > > > > > > > > > Hmm, then for the old driver, it should also do the most update in > > > > > non-atomic context? > > > > > > > > > > One other thing is, I profiled that updating a 3MB shadow buffer needs > > > > > 20 ms, which transfer to 150 MB/s bandwidth. Could it be related with > > > > > the cache setting of DRM shadow buffer? say the orginal code use a > > > > > cachable buffer? > > > > > > > > Hm, that would indicate the write-combining got broken somewhere. This > > > > should definitely be faster. Also we shouldn't transfer the hole > > > > thing, except when scrolling ... > > > > > > First rule of fbcon usage, you are always effectively scrolling. > > > > > > Also these devices might be on a PCIE 1x piece of wet string, not sure > > > if the numbers reflect that. > > > > pcie 1x 1.0 is 250MB/s, so yeah with a bit of inefficiency and > > overhead not entirely out of the question that 150MB/s is actually the > > hw limit. If it's really pcie 1x 1.0, no idea where to check that. > > Also might be worth to double-check that the gpu pci bar is listed as > > wc in debugfs/x86/pat_memtype_list. > > Here is some dump of the device info and the pat_memtype_list, while it is > running other 0day task: Looks all good, I guess Dave is right with this probably only being a real slow, real old pcie link, plus maybe some inefficiencies in the mapping. Your 150MB/s, was that just the copy, or did you include all the setup/map/unmap/teardown too in your measurement in the trace? -Daniel > > controller info > = > 03:00.0 VGA compatible controller: Matrox Electronics Systems Ltd. MGA G200e > [Pilot] ServerEngines (SEP1) (rev 05) (prog-if 00 [VGA controller]) > Subsystem: Intel Corporation MGA G200e [Pilot] ServerEngines (SEP1) > Control: I/O+ Mem+ BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr- > Stepping- SERR- FastB2B- DisINTx- > Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- > SERR- Interrupt: pin A routed to IRQ 16 > NUMA node: 0 > Region 0: Memory at d000 (32-bit, prefetchable) [size=16M] > Region 1: Memory at d180 (32-bit, non-prefetchable) [size=16K] > Region 2: Memory at d100 (32-bit, non-prefetchable) [size=8M] > Expansion ROM at 000c [disabled] [size=128K] > Capabilities: [dc] Power Management version 2 > Flags: PMEClk- DSI+ D1- D2- AuxCurrent=0mA > PME(D0-,D1-,D2-,D3hot-,D3cold-) > Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=0 PME- > Capabilities: [e4] Express (v1) Legacy Endpoint, MSI 00 > DevCap: MaxPayload 256 bytes, PhantFunc 0, Latency L0s <64ns, > L1 <1us > ExtTag- AttnBtn- AttnInd- PwrInd- RBE- FLReset- > DevCtl: Report errors: Correctable- Non-Fatal- Fatal- > Unsupported- > RlxdOrd+ ExtTag- PhantFunc- AuxPwr- NoSnoop- >
[PATCH v4 2/3] drm: Introduce change counter to drm_connector
This counter will be used by drm_helper_probe_detect caller to determine if something else had changed except connection status, like for example edid. Hardware specific drivers are responsible for updating this counter when some change is detected to notify the drm part, which can trigger for example hotplug event. Currently there is no way to propagate that to a calling layer, as we send only connection_status update, however as we see with edid the changes can be broader. v2: Added documentation for the new counter. Rename change_counter to epoch_counter. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105540 Signed-off-by: Stanislav Lisovskiy --- drivers/gpu/drm/drm_connector.c| 1 + drivers/gpu/drm/drm_probe_helper.c | 29 +++-- include/drm/drm_connector.h| 3 +++ 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index 4c766624b20d..18b1ad2a4eee 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -246,6 +246,7 @@ int drm_connector_init(struct drm_device *dev, INIT_LIST_HEAD(&connector->modes); mutex_init(&connector->mutex); connector->edid_blob_ptr = NULL; + connector->epoch_counter = 0; connector->tile_blob_ptr = NULL; connector->status = connector_status_unknown; connector->display_info.panel_orientation = diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c index 351cbc40f0f8..5131ae56e676 100644 --- a/drivers/gpu/drm/drm_probe_helper.c +++ b/drivers/gpu/drm/drm_probe_helper.c @@ -777,6 +777,7 @@ bool drm_helper_hpd_irq_event(struct drm_device *dev) struct drm_connector_list_iter conn_iter; enum drm_connector_status old_status; bool changed = false; + uint64_t old_epoch_counter; if (!dev->mode_config.poll_enabled) return false; @@ -790,20 +791,44 @@ bool drm_helper_hpd_irq_event(struct drm_device *dev) old_status = connector->status; + old_epoch_counter = connector->epoch_counter; + + DRM_DEBUG_KMS("[CONNECTOR:%d:%s] Old change counter %llu\n", connector->base.id, + connector->name, + old_epoch_counter); + connector->status = drm_helper_probe_detect(connector, NULL, false); DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %s to %s\n", connector->base.id, connector->name, drm_get_connector_status_name(old_status), drm_get_connector_status_name(connector->status)); - if (old_status != connector->status) + + DRM_DEBUG_KMS("[CONNECTOR:%d:%s] New change counter %llu\n", + connector->base.id, + connector->name, + connector->epoch_counter); + + if (old_status != connector->status) { changed = true; + } + + /* Check changing of edid when a connector status still remains +* as "connector_status_connected". +*/ + if (connector->status == connector_status_connected && + old_status == connector_status_connected && + old_epoch_counter != connector->epoch_counter) { + changed = true; + } } drm_connector_list_iter_end(&conn_iter); mutex_unlock(&dev->mode_config.mutex); - if (changed) + if (changed) { drm_kms_helper_hotplug_event(dev); + DRM_DEBUG_KMS("Sent hotplug event\n"); + } return changed; } diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index 681cb590f952..a3cc7d0927d6 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -1288,6 +1288,9 @@ struct drm_connector { /** @override_edid: has the EDID been overwritten through debugfs for testing? */ bool override_edid; + /** @epoch_counter: used to detect any other changes in connector, besides status */ + uint64_t epoch_counter; + #define DRM_CONNECTOR_MAX_ENCODER 3 /** * @encoder_ids: Valid encoders for this connector. Please only use -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 0/3] Send a hotplug when edid changes
This series introduce to drm a way to determine if something else except connection_status had changed during probing, which can be used by other drivers as well. Another i915 specific part uses this approach to determine if edid had changed without changing the connection status and send a hotplug event. Stanislav Lisovskiy (3): drm: Add helper to compare edids. drm: Introduce change counter to drm_connector drm/i915: Send hotplug event if edid had changed. drivers/gpu/drm/drm_connector.c | 17 + drivers/gpu/drm/drm_edid.c | 36 drivers/gpu/drm/drm_probe_helper.c | 29 ++-- drivers/gpu/drm/i915/display/intel_hotplug.c | 21 +--- include/drm/drm_connector.h | 3 ++ include/drm/drm_edid.h | 9 + 6 files changed, 108 insertions(+), 7 deletions(-) -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 3/3] drm/i915: Send hotplug event if edid had changed.
Added edid checking to dp and hdmi edid setting functions, which are called from detect hooks. The result currently is propagated to calling layer using drm_connector->change_counter(proposed by Daniel Vetter). drm_helper_hpd_irq_event and intel_encoder_hotplug are currently both responsible for checking if this counter or connection status is changed. There are conflicting parts in drm and i915 which attempt to do the same job - drm_helper_hpd_irq_event attempts to check connector status changes and then call hotplug, just as i915_hotplug_work_func, which calls encoder->hotplug hook which in turn calls generic intel_encoder_hotplug function which attempts to probe if output has changed. Looks like both needs to be changed, so added edid checking also to intel_encoder_hotplug function which is called both for hdmi and dp. v2: Renamed change counter to epoch counter. Fixed type name. v3: Fixed rebase conflict v4: Remove duplicate drm_edid_equal checks from hdmi and dp, lets use only once edid property is getting updated and increment epoch counter from there. Also lets now call drm_connector_update_edid_property right after we get edid always to make sure there is a unified way to handle edid change, without having to change tons of source code as currently drm_connector_update_edid_property is called only in certain cases like reprobing and not right after edid is actually updated. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105540 Signed-off-by: Stanislav Lisovskiy --- drivers/gpu/drm/drm_connector.c | 16 +++ drivers/gpu/drm/drm_edid.c | 3 +++ drivers/gpu/drm/drm_probe_helper.c | 2 +- drivers/gpu/drm/i915/display/intel_hotplug.c | 21 +++- 4 files changed, 36 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index 18b1ad2a4eee..828e57a740d7 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -1867,6 +1867,7 @@ int drm_connector_update_edid_property(struct drm_connector *connector, struct drm_device *dev = connector->dev; size_t size = 0; int ret; + const struct edid *old_edid; /* ignore requests to set edid when overridden */ if (connector->override_edid) @@ -1888,6 +1889,20 @@ int drm_connector_update_edid_property(struct drm_connector *connector, else drm_reset_display_info(connector); + if (connector->edid_blob_ptr) { + old_edid = (struct edid *)connector->edid_blob_ptr->data; + if (old_edid) { + if (!drm_edid_are_equal(edid, old_edid)) { + DRM_DEBUG_KMS("[CONNECTOR:%d:%s] Edid was changed.\n", + connector->base.id, connector->name); + + connector->epoch_counter += 1; + DRM_DEBUG_KMS("Updating change counter to %llu\n", + connector->epoch_counter); + } + } + } + drm_object_property_set_value(&connector->base, dev->mode_config.non_desktop_property, connector->display_info.non_desktop); @@ -1898,6 +1913,7 @@ int drm_connector_update_edid_property(struct drm_connector *connector, edid, &connector->base, dev->mode_config.edid_property); + if (ret) return ret; return drm_connector_set_tile_property(connector); diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 6cd086ea6253..7a327aec00d7 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -1803,6 +1803,9 @@ struct edid *drm_get_edid(struct drm_connector *connector, edid = drm_do_get_edid(connector, drm_do_probe_ddc_edid, adapter); if (edid) drm_get_displayid(connector, edid); + + drm_connector_update_edid_property(connector, edid); + return edid; } EXPORT_SYMBOL(drm_get_edid); diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c index 5131ae56e676..d896d6b5d3b4 100644 --- a/drivers/gpu/drm/drm_probe_helper.c +++ b/drivers/gpu/drm/drm_probe_helper.c @@ -813,7 +813,7 @@ bool drm_helper_hpd_irq_event(struct drm_device *dev) changed = true; } - /* Check changing of edid when a connector status still remains + /* Check changing of epoch counter when a connector status still remains * as "connector_status_connected". */ if (connector->status == connector_status_connected && diff --git a/drivers/gpu/drm/i915/display/intel
[PATCH v4 1/3] drm: Add helper to compare edids.
Many drivers would benefit from using drm helper to compare edid, rather than bothering with own implementation. v2: Added documentation for this function. Signed-off-by: Stanislav Lisovskiy --- drivers/gpu/drm/drm_edid.c | 33 + include/drm/drm_edid.h | 9 + 2 files changed, 42 insertions(+) diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 82a4ceed3fcf..6cd086ea6253 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -1362,6 +1362,39 @@ static bool drm_edid_is_zero(const u8 *in_edid, int length) return true; } +/** + * drm_edid_are_equal - compare two edid blobs. + * @edid1: pointer to first blob + * @edid2: pointer to second blob + * This helper can be used during probing to determine if + * edid had changed. + */ +bool drm_edid_are_equal(struct edid *edid1, struct edid *edid2) +{ + int edid1_len, edid2_len; + bool edid1_present = edid1 != NULL; + bool edid2_present = edid2 != NULL; + + if (edid1_present != edid2_present) + return false; + + if (edid1) { + + edid1_len = EDID_LENGTH * (1 + edid1->extensions); + edid2_len = EDID_LENGTH * (1 + edid2->extensions); + + if (edid1_len != edid2_len) + return false; + + if (memcmp(edid1, edid2, edid1_len)) + return false; + } + + return true; +} +EXPORT_SYMBOL(drm_edid_are_equal); + + /** * drm_edid_block_valid - Sanity check the EDID block (base or extension) * @raw_edid: pointer to raw EDID block diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h index b9719418c3d2..716964f63312 100644 --- a/include/drm/drm_edid.h +++ b/include/drm/drm_edid.h @@ -354,6 +354,15 @@ drm_load_edid_firmware(struct drm_connector *connector) } #endif +/** + * drm_edid_are_equal - compare two edid blobs. + * @edid1: pointer to first blob + * @edid2: pointer to second blob + * This helper can be used during probing to determine if + * edid had changed. + */ +bool drm_edid_are_equal(struct edid *edid1, struct edid *edid2); + int drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame, struct drm_connector *connector, -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: damage_helper: Fix race checking plane->state->fb
On Wed, Sep 4, 2019 at 10:29 PM Sean Paul wrote: > > From: Sean Paul > > Since the dirtyfb ioctl doesn't give us any hints as to which plane is > scanning out the fb it's marking as damaged, we need to loop through > planes to find it. > > Currently we just reach into plane state and check, but that can race > with another commit changing the fb out from under us. This patch locks > the plane before checking the fb and will release the lock if the plane > is not displaying the dirty fb. > > Fixes: b9fc5e01d1ce ("drm: Add helper to implement legacy dirtyfb") > Cc: Rob Clark > Cc: Deepak Rawat > Cc: Daniel Vetter > Cc: Thomas Hellstrom > Cc: Maarten Lankhorst > Cc: Maxime Ripard > Cc: Sean Paul > Cc: David Airlie > Cc: Daniel Vetter > Cc: dri-devel@lists.freedesktop.org > Cc: # v5.0+ > Reported-by: Daniel Vetter > Signed-off-by: Sean Paul > --- > drivers/gpu/drm/drm_damage_helper.c | 8 +++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_damage_helper.c > b/drivers/gpu/drm/drm_damage_helper.c > index 8230dac01a89..3a4126dc2520 100644 > --- a/drivers/gpu/drm/drm_damage_helper.c > +++ b/drivers/gpu/drm/drm_damage_helper.c > @@ -212,8 +212,14 @@ int drm_atomic_helper_dirtyfb(struct drm_framebuffer *fb, > drm_for_each_plane(plane, fb->dev) { > struct drm_plane_state *plane_state; > > - if (plane->state->fb != fb) > + ret = drm_modeset_lock(&plane->mutex, state->acquire_ctx); > + if (ret) I think for paranoid safety we should have a WARN_ON(ret == -EALREADY) here. It should be impossible, but if it's not for some oddball reason, we'll blow up. With that: Reviewed-by: Daniel Vetter But please give this a spin with some workloads and the ww_mutex slowpath debugging enabled, just to makre sure. -Daniel > + goto out; > + > + if (plane->state->fb != fb) { > + drm_modeset_unlock(&plane->mutex); > continue; > + } > > plane_state = drm_atomic_get_plane_state(state, plane); > if (IS_ERR(plane_state)) { > -- > Sean Paul, Software Engineer, Google / Chromium OS > -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 0/4] Have TTM support SEV encryption with coherent memory
On 9/3/19 3:15 PM, Thomas Hellström (VMware) wrote: With SEV memory encryption and in some cases also with SME memory encryption, coherent memory is unencrypted. In those cases, TTM doesn't set up the correct page protection. Fix this by having the TTM coherent page allocator call into the platform code to determine whether coherent memory is encrypted or not, and modify the page protection if it is not. v2: - Use force_dma_unencrypted() rather than sev_active() to catch also the special SME encryption cases. So, this patchset is obviously withdrawn since a) We shouldn't have TTM shortcut the dma API in this way. b) To reviewers it was pretty unclear why this was needed in the first place, and became even more unclear in the context of the TTM fault handler. I've just send out an RFC patchset that basically does the same but in the context of dma_mmap_coherent() I hope this clears things up and we should hopefully be able to use a new dma API function from within the TTM fault handler. Thanks, Thomas ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [LKP] [drm/mgag200] 90f479ae51: vm-scalability.median -18.8% regression
On Thu, Sep 05, 2019 at 06:37:47PM +0800, Daniel Vetter wrote: > On Thu, Sep 5, 2019 at 8:58 AM Feng Tang wrote: > > > > Hi Vetter, > > > > On Wed, Sep 04, 2019 at 01:20:29PM +0200, Daniel Vetter wrote: > > > On Wed, Sep 4, 2019 at 1:15 PM Dave Airlie wrote: > > > > > > > > On Wed, 4 Sep 2019 at 19:17, Daniel Vetter wrote: > > > > > > > > > > On Wed, Sep 4, 2019 at 10:35 AM Feng Tang wrote: > > > > > > > > > > > > Hi Daniel, > > > > > > > > > > > > On Wed, Sep 04, 2019 at 10:11:11AM +0200, Daniel Vetter wrote: > > > > > > > On Wed, Sep 4, 2019 at 8:53 AM Thomas Zimmermann > > > > > > > wrote: > > > > > > > > > > > > > > > > Hi > > > > > > > > > > > > > > > > Am 04.09.19 um 08:27 schrieb Feng Tang: > > > > > > > > >> Thank you for testing. But don't get too excited, because > > > > > > > > >> the patch > > > > > > > > >> simulates a bug that was present in the original mgag200 > > > > > > > > >> code. A > > > > > > > > >> significant number of frames are simply skipped. That is > > > > > > > > >> apparently the > > > > > > > > >> reason why it's faster. > > > > > > > > > > > > > > > > > > Thanks for the detailed info, so the original code skips > > > > > > > > > time-consuming > > > > > > > > > work inside atomic context on purpose. Is there any space to > > > > > > > > > optmise it? > > > > > > > > > If 2 scheduled update worker are handled at almost same time, > > > > > > > > > can one be > > > > > > > > > skipped? > > > > > > > > > > > > > > > > To my knowledge, there's only one instance of the worker. > > > > > > > > Re-scheduling > > > > > > > > the worker before a previous instance started, will not create > > > > > > > > a second > > > > > > > > instance. The worker's instance will complete all pending > > > > > > > > updates. So in > > > > > > > > some way, skipping workers already happens. > > > > > > > > > > > > > > So I think that the most often fbcon update from atomic context > > > > > > > is the > > > > > > > blinking cursor. If you disable that one you should be back to > > > > > > > the old > > > > > > > performance level I think, since just writing to dmesg is from > > > > > > > process > > > > > > > context, so shouldn't change. > > > > > > > > > > > > Hmm, then for the old driver, it should also do the most update in > > > > > > non-atomic context? > > > > > > > > > > > > One other thing is, I profiled that updating a 3MB shadow buffer > > > > > > needs > > > > > > 20 ms, which transfer to 150 MB/s bandwidth. Could it be related > > > > > > with > > > > > > the cache setting of DRM shadow buffer? say the orginal code use a > > > > > > cachable buffer? > > > > > > > > > > Hm, that would indicate the write-combining got broken somewhere. This > > > > > should definitely be faster. Also we shouldn't transfer the hole > > > > > thing, except when scrolling ... > > > > > > > > First rule of fbcon usage, you are always effectively scrolling. > > > > > > > > Also these devices might be on a PCIE 1x piece of wet string, not sure > > > > if the numbers reflect that. > > > > > > pcie 1x 1.0 is 250MB/s, so yeah with a bit of inefficiency and > > > overhead not entirely out of the question that 150MB/s is actually the > > > hw limit. If it's really pcie 1x 1.0, no idea where to check that. > > > Also might be worth to double-check that the gpu pci bar is listed as > > > wc in debugfs/x86/pat_memtype_list. > > > > Here is some dump of the device info and the pat_memtype_list, while it is > > running other 0day task: > > Looks all good, I guess Dave is right with this probably only being a > real slow, real old pcie link, plus maybe some inefficiencies in the > mapping. Your 150MB/s, was that just the copy, or did you include all > the setup/map/unmap/teardown too in your measurement in the trace? Following is the breakdown, the 19240 us is the memory copy time The drm_fb_helper_dirty_work() calls sequentially 1. drm_client_buffer_vmap (290 us) 2. drm_fb_helper_dirty_blit_real (19240 us) 3. helper->fb->funcs->dirty()---> NULL for mgag200 driver 4. drm_client_buffer_vunmap (215 us) Thanks, Feng > -Daniel > > > > > controller info > > = > > 03:00.0 VGA compatible controller: Matrox Electronics Systems Ltd. MGA > > G200e [Pilot] ServerEngines (SEP1) (rev 05) (prog-if 00 [VGA controller]) > > Subsystem: Intel Corporation MGA G200e [Pilot] ServerEngines (SEP1) > > Control: I/O+ Mem+ BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr- > > Stepping- SERR- FastB2B- DisINTx- > > Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- > > SERR- > Interrupt: pin A routed to IRQ 16 > > NUMA node: 0 > > Region 0: Memory at d000 (32-bit, prefetchable) [size=16M] > > Region 1: Memory at d180 (32-bit, non-prefetchable) [size=16K] > > Region 2: Memory at d100 (32-bit, non-prefetchable) [size=8M] > > Expansion ROM at 000c [disabled] [size=128K] > > Capabi
[PATCH v5 0/3] Send a hotplug when edid changes
This series introduce to drm a way to determine if something else except connection_status had changed during probing, which can be used by other drivers as well. Another i915 specific part uses this approach to determine if edid had changed without changing the connection status and send a hotplug event. Stanislav Lisovskiy (3): drm: Add helper to compare edids. drm: Introduce change counter to drm_connector drm/i915: Send hotplug event if edid had changed. drivers/gpu/drm/drm_connector.c | 16 + drivers/gpu/drm/drm_edid.c | 36 drivers/gpu/drm/drm_probe_helper.c | 29 ++-- drivers/gpu/drm/i915/display/intel_hotplug.c | 21 +--- include/drm/drm_connector.h | 3 ++ include/drm/drm_edid.h | 9 + 6 files changed, 107 insertions(+), 7 deletions(-) -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 1/3] drm: Add helper to compare edids.
Many drivers would benefit from using drm helper to compare edid, rather than bothering with own implementation. v2: Added documentation for this function. Signed-off-by: Stanislav Lisovskiy --- drivers/gpu/drm/drm_edid.c | 33 + include/drm/drm_edid.h | 9 + 2 files changed, 42 insertions(+) diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 82a4ceed3fcf..6cd086ea6253 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -1362,6 +1362,39 @@ static bool drm_edid_is_zero(const u8 *in_edid, int length) return true; } +/** + * drm_edid_are_equal - compare two edid blobs. + * @edid1: pointer to first blob + * @edid2: pointer to second blob + * This helper can be used during probing to determine if + * edid had changed. + */ +bool drm_edid_are_equal(struct edid *edid1, struct edid *edid2) +{ + int edid1_len, edid2_len; + bool edid1_present = edid1 != NULL; + bool edid2_present = edid2 != NULL; + + if (edid1_present != edid2_present) + return false; + + if (edid1) { + + edid1_len = EDID_LENGTH * (1 + edid1->extensions); + edid2_len = EDID_LENGTH * (1 + edid2->extensions); + + if (edid1_len != edid2_len) + return false; + + if (memcmp(edid1, edid2, edid1_len)) + return false; + } + + return true; +} +EXPORT_SYMBOL(drm_edid_are_equal); + + /** * drm_edid_block_valid - Sanity check the EDID block (base or extension) * @raw_edid: pointer to raw EDID block diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h index b9719418c3d2..716964f63312 100644 --- a/include/drm/drm_edid.h +++ b/include/drm/drm_edid.h @@ -354,6 +354,15 @@ drm_load_edid_firmware(struct drm_connector *connector) } #endif +/** + * drm_edid_are_equal - compare two edid blobs. + * @edid1: pointer to first blob + * @edid2: pointer to second blob + * This helper can be used during probing to determine if + * edid had changed. + */ +bool drm_edid_are_equal(struct edid *edid1, struct edid *edid2); + int drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame, struct drm_connector *connector, -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 2/3] drm: Introduce change counter to drm_connector
This counter will be used by drm_helper_probe_detect caller to determine if something else had changed except connection status, like for example edid. Hardware specific drivers are responsible for updating this counter when some change is detected to notify the drm part, which can trigger for example hotplug event. Currently there is no way to propagate that to a calling layer, as we send only connection_status update, however as we see with edid the changes can be broader. v2: Added documentation for the new counter. Rename change_counter to epoch_counter. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105540 Signed-off-by: Stanislav Lisovskiy --- drivers/gpu/drm/drm_connector.c| 1 + drivers/gpu/drm/drm_probe_helper.c | 29 +++-- include/drm/drm_connector.h| 3 +++ 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index 4c766624b20d..18b1ad2a4eee 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -246,6 +246,7 @@ int drm_connector_init(struct drm_device *dev, INIT_LIST_HEAD(&connector->modes); mutex_init(&connector->mutex); connector->edid_blob_ptr = NULL; + connector->epoch_counter = 0; connector->tile_blob_ptr = NULL; connector->status = connector_status_unknown; connector->display_info.panel_orientation = diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c index 351cbc40f0f8..5131ae56e676 100644 --- a/drivers/gpu/drm/drm_probe_helper.c +++ b/drivers/gpu/drm/drm_probe_helper.c @@ -777,6 +777,7 @@ bool drm_helper_hpd_irq_event(struct drm_device *dev) struct drm_connector_list_iter conn_iter; enum drm_connector_status old_status; bool changed = false; + uint64_t old_epoch_counter; if (!dev->mode_config.poll_enabled) return false; @@ -790,20 +791,44 @@ bool drm_helper_hpd_irq_event(struct drm_device *dev) old_status = connector->status; + old_epoch_counter = connector->epoch_counter; + + DRM_DEBUG_KMS("[CONNECTOR:%d:%s] Old change counter %llu\n", connector->base.id, + connector->name, + old_epoch_counter); + connector->status = drm_helper_probe_detect(connector, NULL, false); DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %s to %s\n", connector->base.id, connector->name, drm_get_connector_status_name(old_status), drm_get_connector_status_name(connector->status)); - if (old_status != connector->status) + + DRM_DEBUG_KMS("[CONNECTOR:%d:%s] New change counter %llu\n", + connector->base.id, + connector->name, + connector->epoch_counter); + + if (old_status != connector->status) { changed = true; + } + + /* Check changing of edid when a connector status still remains +* as "connector_status_connected". +*/ + if (connector->status == connector_status_connected && + old_status == connector_status_connected && + old_epoch_counter != connector->epoch_counter) { + changed = true; + } } drm_connector_list_iter_end(&conn_iter); mutex_unlock(&dev->mode_config.mutex); - if (changed) + if (changed) { drm_kms_helper_hotplug_event(dev); + DRM_DEBUG_KMS("Sent hotplug event\n"); + } return changed; } diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index 681cb590f952..a3cc7d0927d6 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -1288,6 +1288,9 @@ struct drm_connector { /** @override_edid: has the EDID been overwritten through debugfs for testing? */ bool override_edid; + /** @epoch_counter: used to detect any other changes in connector, besides status */ + uint64_t epoch_counter; + #define DRM_CONNECTOR_MAX_ENCODER 3 /** * @encoder_ids: Valid encoders for this connector. Please only use -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 3/3] drm/i915: Send hotplug event if edid had changed.
Added edid checking to dp and hdmi edid setting functions, which are called from detect hooks. The result currently is propagated to calling layer using drm_connector->change_counter(proposed by Daniel Vetter). drm_helper_hpd_irq_event and intel_encoder_hotplug are currently both responsible for checking if this counter or connection status is changed. There are conflicting parts in drm and i915 which attempt to do the same job - drm_helper_hpd_irq_event attempts to check connector status changes and then call hotplug, just as i915_hotplug_work_func, which calls encoder->hotplug hook which in turn calls generic intel_encoder_hotplug function which attempts to probe if output has changed. Looks like both needs to be changed, so added edid checking also to intel_encoder_hotplug function which is called both for hdmi and dp. v2: Renamed change counter to epoch counter. Fixed type name. v3: Fixed rebase conflict v4: Remove duplicate drm_edid_equal checks from hdmi and dp, lets use only once edid property is getting updated and increment epoch counter from there. Also lets now call drm_connector_update_edid_property right after we get edid always to make sure there is a unified way to handle edid change, without having to change tons of source code as currently drm_connector_update_edid_property is called only in certain cases like reprobing and not right after edid is actually updated. v5: Fixed const modifiers, removed blank line Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105540 Signed-off-by: Stanislav Lisovskiy --- drivers/gpu/drm/drm_connector.c | 15 ++ drivers/gpu/drm/drm_edid.c | 5 - drivers/gpu/drm/drm_probe_helper.c | 2 +- drivers/gpu/drm/i915/display/intel_hotplug.c | 21 +++- include/drm/drm_edid.h | 2 +- 5 files changed, 37 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index 18b1ad2a4eee..98fd236acc57 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -1867,6 +1867,7 @@ int drm_connector_update_edid_property(struct drm_connector *connector, struct drm_device *dev = connector->dev; size_t size = 0; int ret; + const struct edid *old_edid; /* ignore requests to set edid when overridden */ if (connector->override_edid) @@ -1888,6 +1889,20 @@ int drm_connector_update_edid_property(struct drm_connector *connector, else drm_reset_display_info(connector); + if (connector->edid_blob_ptr) { + old_edid = (const struct edid *)connector->edid_blob_ptr->data; + if (old_edid) { + if (!drm_edid_are_equal(edid, old_edid)) { + DRM_DEBUG_KMS("[CONNECTOR:%d:%s] Edid was changed.\n", + connector->base.id, connector->name); + + connector->epoch_counter += 1; + DRM_DEBUG_KMS("Updating change counter to %llu\n", + connector->epoch_counter); + } + } + } + drm_object_property_set_value(&connector->base, dev->mode_config.non_desktop_property, connector->display_info.non_desktop); diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 6cd086ea6253..48b716abc9ef 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -1369,7 +1369,7 @@ static bool drm_edid_is_zero(const u8 *in_edid, int length) * This helper can be used during probing to determine if * edid had changed. */ -bool drm_edid_are_equal(struct edid *edid1, struct edid *edid2) +bool drm_edid_are_equal(const struct edid *edid1, const struct edid *edid2) { int edid1_len, edid2_len; bool edid1_present = edid1 != NULL; @@ -1803,6 +1803,9 @@ struct edid *drm_get_edid(struct drm_connector *connector, edid = drm_do_get_edid(connector, drm_do_probe_ddc_edid, adapter); if (edid) drm_get_displayid(connector, edid); + + drm_connector_update_edid_property(connector, edid); + return edid; } EXPORT_SYMBOL(drm_get_edid); diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c index 5131ae56e676..d896d6b5d3b4 100644 --- a/drivers/gpu/drm/drm_probe_helper.c +++ b/drivers/gpu/drm/drm_probe_helper.c @@ -813,7 +813,7 @@ bool drm_helper_hpd_irq_event(struct drm_device *dev) changed = true; } - /* Check changing of edid when a connector status still remains + /* Check changing of epoch counter when a connector status still remains * as "connector_status_connected". */ i
Re: [PATCH v3 14/14] drm: rcar-du: Force CMM enablement when resuming
Hi Laurent, On Tue, Aug 27, 2019 at 03:05:17AM +0300, Laurent Pinchart wrote: > Hi Jacopo, > > (Question for Daniel below) > > Thank you for the patch. > > On Sun, Aug 25, 2019 at 03:51:54PM +0200, Jacopo Mondi wrote: > > When resuming from system suspend, the DU driver is responsible for > > reprogramming and enabling the CMM unit if it was in use at the time > > the system entered the suspend state. > > > > Force the color_mgmt_changed flag to true if any of the DRM color > > transformation properties was set in the CRTC state duplicated at > > suspend time, as the CMM gets reprogrammed only if said flag is active in > > the rcar_du_atomic_commit_update_cmm() method. > > > > Signed-off-by: Jacopo Mondi > > --- > > drivers/gpu/drm/rcar-du/rcar_du_drv.c | 21 + > > 1 file changed, 21 insertions(+) > > > > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c > > b/drivers/gpu/drm/rcar-du/rcar_du_drv.c > > index 018480a8f35c..6e38495fb78f 100644 > > --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c > > +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c > > @@ -17,6 +17,7 @@ > > #include > > #include > > > > +#include > > #include > > #include > > #include > > @@ -482,6 +483,26 @@ static int rcar_du_pm_suspend(struct device *dev) > > static int rcar_du_pm_resume(struct device *dev) > > { > > struct rcar_du_device *rcdu = dev_get_drvdata(dev); > > + struct drm_atomic_state *state = rcdu->ddev->mode_config.suspend_state; > > + unsigned int i; > > + > > + for (i = 0; i < rcdu->num_crtcs; ++i) { > > + struct drm_crtc *crtc = &rcdu->crtcs[i].crtc; > > + struct drm_crtc_state *crtc_state; > > + > > + crtc_state = drm_atomic_get_existing_crtc_state(state, crtc); > > + if (!crtc_state) > > + continue; > > Shouldn't you get the new state here ? > I have followed the drm_atomic_helper_suspend() call stack, that calls drm_atomic_helper_duplicate_state() which then assign the crtct state with drm_atomic_get_crtc_state(), where I read: crtc_state = drm_atomic_get_existing_crtc_state(state, crtc); ... state->crtcs[index].state = crtc_state; state->crtcs[index].old_state = crtc->state; state->crtcs[index].new_state = crtc_state; So state or new_state for the purpose of getting back the crtc state are the same if I'm not mistaken. > > + > > + /* > > +* Force re-enablement of CMM after system resume if any > > +* of the DRM color transformation properties was set in > > +* the state saved at system suspend time. > > +*/ > > + if (crtc_state->gamma_lut || crtc_state->degamma_lut || > > + crtc_state->ctm) > > We don't support degamma_lut or crm, so I would drop those. yeah, I added them as it was less code to change when we'll support them. But for now they could be removed. > > With these small issues addressed, > > Reviewed-by: Laurent Pinchart > > Shouldn't we however squash this with the previous patch to avoid > bisection issues ? > Which one in your opinion? "drm: rcar-du: kms: Update CMM in atomic commit tail" ? It seems to me they do quite different things though.. Thanks j > > + crtc_state->color_mgmt_changed = true; > > Daniel, is this something that would make sense in the KMS core (or > helpers) ? > > > + } > > > > return drm_mode_config_helper_resume(rcdu->ddev); > > } > > -- > Regards, > > Laurent Pinchart signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 02/19] drm/atomic-helper: Make crtc helper funcs optional
On Thu, Sep 05, 2019 at 06:38:30AM +, Lisovskiy, Stanislav wrote: > On Mon, 2019-07-08 at 15:53 +0300, Ville Syrjala wrote: > > From: Ville Syrjälä > > > > Allow drivers to call drm_atomic_helper_check_modeset() without > > having the crtc helper funcs specified. i915 doesn't need those > > anymore. > > > > Signed-off-by: Ville Syrjälä > > --- > > drivers/gpu/drm/drm_atomic_helper.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c > > b/drivers/gpu/drm/drm_atomic_helper.c > > index aa16ea17ff9b..fb2ce692ae5b 100644 > > --- a/drivers/gpu/drm/drm_atomic_helper.c > > +++ b/drivers/gpu/drm/drm_atomic_helper.c > > @@ -481,7 +481,7 @@ mode_fixup(struct drm_atomic_state *state) > > continue; > > > > funcs = crtc->helper_private; > > - if (!funcs->mode_fixup) > > + if (!funcs || !funcs->mode_fixup) > > continue; > > > > ret = funcs->mode_fixup(crtc, &new_crtc_state->mode, > > But it still won't allow to call anything from here, if no funcs or no > mode_fixup is specified. Anyway seems to be a good idea to check both > funcs or funcs->mode_fixup being NULL, just don't get the commit > message a bit :) NULL->mode_fixup will oops. -- Ville Syrjälä Intel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 2/3] drm: Introduce change counter to drm_connector
Op 05-09-2019 om 12:37 schreef Stanislav Lisovskiy: > This counter will be used by drm_helper_probe_detect caller to determine > if something else had changed except connection status, > like for example edid. Hardware specific drivers are responsible > for updating this counter when some change is detected to notify > the drm part, which can trigger for example hotplug event. > > Currently there is no way to propagate that to a calling layer, > as we send only connection_status update, however as we see with > edid the changes can be broader. > > v2: Added documentation for the new counter. Rename change_counter to > epoch_counter. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105540 > Signed-off-by: Stanislav Lisovskiy > --- > drivers/gpu/drm/drm_connector.c| 1 + > drivers/gpu/drm/drm_probe_helper.c | 29 +++-- > include/drm/drm_connector.h| 3 +++ > 3 files changed, 31 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c > index 4c766624b20d..18b1ad2a4eee 100644 > --- a/drivers/gpu/drm/drm_connector.c > +++ b/drivers/gpu/drm/drm_connector.c > @@ -246,6 +246,7 @@ int drm_connector_init(struct drm_device *dev, > INIT_LIST_HEAD(&connector->modes); > mutex_init(&connector->mutex); > connector->edid_blob_ptr = NULL; > + connector->epoch_counter = 0; > connector->tile_blob_ptr = NULL; > connector->status = connector_status_unknown; > connector->display_info.panel_orientation = > diff --git a/drivers/gpu/drm/drm_probe_helper.c > b/drivers/gpu/drm/drm_probe_helper.c > index 351cbc40f0f8..5131ae56e676 100644 > --- a/drivers/gpu/drm/drm_probe_helper.c > +++ b/drivers/gpu/drm/drm_probe_helper.c > @@ -777,6 +777,7 @@ bool drm_helper_hpd_irq_event(struct drm_device *dev) > struct drm_connector_list_iter conn_iter; > enum drm_connector_status old_status; > bool changed = false; > + uint64_t old_epoch_counter; > > if (!dev->mode_config.poll_enabled) > return false; > @@ -790,20 +791,44 @@ bool drm_helper_hpd_irq_event(struct drm_device *dev) > > old_status = connector->status; > > + old_epoch_counter = connector->epoch_counter; > + > + DRM_DEBUG_KMS("[CONNECTOR:%d:%s] Old change counter %llu\n", > connector->base.id, > + connector->name, > + old_epoch_counter); > + > connector->status = drm_helper_probe_detect(connector, NULL, > false); Pass a u32 *changed as argument to drm_helper_probe_detect? would require signature change, > DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %s to > %s\n", > connector->base.id, > connector->name, > drm_get_connector_status_name(old_status), > drm_get_connector_status_name(connector->status)); > - if (old_status != connector->status) > + > + DRM_DEBUG_KMS("[CONNECTOR:%d:%s] New change counter %llu\n", > + connector->base.id, > + connector->name, > + connector->epoch_counter); > + > + if (old_status != connector->status) { > changed = true; > + } > + > + /* Check changing of edid when a connector status still remains > + * as "connector_status_connected". > + */ > + if (connector->status == connector_status_connected && > + old_status == connector_status_connected && > + old_epoch_counter != connector->epoch_counter) { > + changed = true; > + } Could we bump the epoch counter for any event being sent out? Would make more sense. > } > drm_connector_list_iter_end(&conn_iter); > mutex_unlock(&dev->mode_config.mutex); > > - if (changed) > + if (changed) { > drm_kms_helper_hotplug_event(dev); > + DRM_DEBUG_KMS("Sent hotplug event\n"); > + } > > return changed; > } > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h > index 681cb590f952..a3cc7d0927d6 100644 > --- a/include/drm/drm_connector.h > +++ b/include/drm/drm_connector.h > @@ -1288,6 +1288,9 @@ struct drm_connector { > /** @override_edid: has the EDID been overwritten through debugfs for > testing? */ > bool override_edid; > > + /** @epoch_counter: used to detect any other changes in connector, > besides status */ > + uint64_t epoch_counter; > + > #define DRM_CONNECTOR_MAX_ENCODER 3 > /** >* @encoder_ids: Valid encoders for this connector. Please only use ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 109380] [CI][BAT] igt@kms_chamelium@*- warn/fail - Last errno: 113, No route to host
https://bugs.freedesktop.org/show_bug.cgi?id=109380 --- Comment #8 from Arek Hiler --- Merged and fixed, issues not seen in 2 weekd :-) -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 111481] AMD Navi GPU frequent freezes on both Manjaro/Ubuntu with kernel 5.3 and mesa 19.2 -git/llvm9
https://bugs.freedesktop.org/show_bug.cgi?id=111481 --- Comment #29 from Marko Popovic --- (In reply to Mathieu Belanger from comment #27) > It did fix it for me too. (In reply to Pierre-Eric Pelloux-Prayer from comment #28) > Regarding sdma ring hangs: if you still have access to the affected machine > using ssh, it would be helpful to add a comment with the following > information: > > - the last dmesg lines (at least the "[drm:amdgpu_job_timedout [amdgpu]] > *ERROR* ring sdma1 timeout, signaled seq=9871, emitted seq=9873" one) > - the output of : umr -R sdma0 (or sdma1 depending on which one failed) > > Thanks! Mathieu could you assist Pierre-Eric with this? I am currently on vacation and won't be able to debug or test further until 15th of September. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/msm/adreno: Do not print error on "qcom, gpu-pwrlevels" absence
Booting the adreno driver on a imx53 board leads to the following error message: adreno 3000.gpu: [drm:adreno_gpu_init] *ERROR* Could not find the GPU powerlevels As the "qcom,gpu-pwrlevels" property is optional and never present on i.MX5, turn the message into debug level instead. Signed-off-by: Fabio Estevam --- drivers/gpu/drm/msm/adreno/adreno_gpu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c index 048c8be426f3..73c79f1614c1 100644 --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c @@ -825,7 +825,7 @@ static int adreno_get_legacy_pwrlevels(struct device *dev) node = of_get_compatible_child(dev->of_node, "qcom,gpu-pwrlevels"); if (!node) { - DRM_DEV_ERROR(dev, "Could not find the GPU powerlevels\n"); + DRM_DEV_DEBUG(dev, "Could not find the GPU powerlevels\n"); return -ENXIO; } -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 08/14] drm: rcar-du: Add support for CMM
Hi Jacopo, On Thu, Sep 05, 2019 at 11:57:57AM +0200, Jacopo Mondi wrote: > On Tue, Aug 27, 2019 at 07:34:23PM +0300, Laurent Pinchart wrote: > > On Tue, Aug 27, 2019 at 04:56:19PM +0200, Jacopo Mondi wrote: > >> On Tue, Aug 27, 2019 at 03:24:22AM +0300, Laurent Pinchart wrote: > >>> On Sun, Aug 25, 2019 at 03:51:48PM +0200, Jacopo Mondi wrote: > Add a driver for the R-Car Display Unit Color Correction Module. > > In most of Gen3 SoCs, each DU output channel is provided with a CMM unit > to perform image enhancement and color correction. > > Add support for CMM through a driver that supports configuration of > the 1-dimensional LUT table. More advanced CMM feature will be > implemented on top of this basic one. > > Signed-off-by: Jacopo Mondi > --- > drivers/gpu/drm/rcar-du/Kconfig| 7 + > drivers/gpu/drm/rcar-du/Makefile | 1 + > drivers/gpu/drm/rcar-du/rcar_cmm.c | 262 + > drivers/gpu/drm/rcar-du/rcar_cmm.h | 38 + > 4 files changed, 308 insertions(+) > create mode 100644 drivers/gpu/drm/rcar-du/rcar_cmm.c > create mode 100644 drivers/gpu/drm/rcar-du/rcar_cmm.h > > diff --git a/drivers/gpu/drm/rcar-du/Kconfig > b/drivers/gpu/drm/rcar-du/Kconfig > index 1529849e217e..539d232790d1 100644 > --- a/drivers/gpu/drm/rcar-du/Kconfig > +++ b/drivers/gpu/drm/rcar-du/Kconfig > @@ -13,6 +13,13 @@ config DRM_RCAR_DU > Choose this option if you have an R-Car chipset. > If M is selected the module will be called rcar-du-drm. > > +config DRM_RCAR_CMM > +bool "R-Car DU Color Management Module (CMM) Support" > +depends on DRM && OF > +depends on DRM_RCAR_DU > +help > + Enable support for R-Car Color Management Module (CMM). > + > config DRM_RCAR_DW_HDMI > tristate "R-Car DU Gen3 HDMI Encoder Support" > depends on DRM && OF > diff --git a/drivers/gpu/drm/rcar-du/Makefile > b/drivers/gpu/drm/rcar-du/Makefile > index 6c2ed9c46467..4d1187ccc3e5 100644 > --- a/drivers/gpu/drm/rcar-du/Makefile > +++ b/drivers/gpu/drm/rcar-du/Makefile > @@ -15,6 +15,7 @@ rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS)+= rcar_du_of.o > \ > rcar-du-drm-$(CONFIG_DRM_RCAR_VSP) += rcar_du_vsp.o > rcar-du-drm-$(CONFIG_DRM_RCAR_WRITEBACK) += rcar_du_writeback.o > > +obj-$(CONFIG_DRM_RCAR_CMM) += rcar_cmm.o > obj-$(CONFIG_DRM_RCAR_DU) += rcar-du-drm.o > obj-$(CONFIG_DRM_RCAR_DW_HDMI) += rcar_dw_hdmi.o > obj-$(CONFIG_DRM_RCAR_LVDS) += rcar_lvds.o > diff --git a/drivers/gpu/drm/rcar-du/rcar_cmm.c > b/drivers/gpu/drm/rcar-du/rcar_cmm.c > new file mode 100644 > index ..55361f5701e8 > --- /dev/null > +++ b/drivers/gpu/drm/rcar-du/rcar_cmm.c > @@ -0,0 +1,262 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * rcar_cmm.c -- R-Car Display Unit Color Management Module > + * > + * Copyright (C) 2019 Jacopo Mondi > + */ > + > +#include > +#include > +#include > +#include > +#include > + > +#include > + > +#include "rcar_cmm.h" > + > +#define CM2_LUT_CTRL0x > +#define CM2_LUT_CTRL_LUT_EN BIT(0) > +#define CM2_LUT_TBL_BASE0x0600 > +#define CM2_LUT_TBL(__i)(CM2_LUT_TBL_BASE + (__i) * 4) > + > +struct rcar_cmm { > +void __iomem *base; > +bool enabled; > + > +/* > + * @lut:1D-LUT status > + * @lut.enabled:1D-LUT enabled flag > + * @lut.size: Number of entries in the LUT table > >>> > >>> Please see my review of patch 13/14, I wonder if we could drop this > >>> field. > >>> > + * @lut.table: Table of 1D-LUT entries scaled to HW > support > + * precision (8-bits per color component) > + */ > +struct { > +bool enabled; > +unsigned int size; > +u32 table[CMM_GAMMA_LUT_SIZE]; > +} lut; > +}; > + > +static inline int rcar_cmm_read(struct rcar_cmm *rcmm, u32 reg) > +{ > +return ioread32(rcmm->base + reg); > +} > + > +static inline void rcar_cmm_write(struct rcar_cmm *rcmm, u32 reg, u32 > data) > +{ > +iowrite32(data, rcmm->base + reg); > +} > + > +/* > + * rcar_cmm_lut_extract() - Scale down to hw precision the DRM LUT table > >>> > >>> s/hw/hardware/ (and below too) > >>> > + * entries and store them. > + * @rcmm: Pointer to the CMM de
Re: [PATCH v4 2/3] drm: Introduce change counter to drm_connector
On Thu, 2019-09-05 at 13:01 +0200, Maarten Lankhorst wrote: > Op 05-09-2019 om 12:37 schreef Stanislav Lisovskiy: > > This counter will be used by drm_helper_probe_detect caller to > > determine > > if something else had changed except connection status, > > like for example edid. Hardware specific drivers are responsible > > for updating this counter when some change is detected to notify > > the drm part, which can trigger for example hotplug event. > > > > Currently there is no way to propagate that to a calling layer, > > as we send only connection_status update, however as we see with > > edid the changes can be broader. > > > > v2: Added documentation for the new counter. Rename change_counter > > to > > epoch_counter. > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105540 > > Signed-off-by: Stanislav Lisovskiy > > --- > > drivers/gpu/drm/drm_connector.c| 1 + > > drivers/gpu/drm/drm_probe_helper.c | 29 > > +++-- > > include/drm/drm_connector.h| 3 +++ > > 3 files changed, 31 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_connector.c > > b/drivers/gpu/drm/drm_connector.c > > index 4c766624b20d..18b1ad2a4eee 100644 > > --- a/drivers/gpu/drm/drm_connector.c > > +++ b/drivers/gpu/drm/drm_connector.c > > @@ -246,6 +246,7 @@ int drm_connector_init(struct drm_device *dev, > > INIT_LIST_HEAD(&connector->modes); > > mutex_init(&connector->mutex); > > connector->edid_blob_ptr = NULL; > > + connector->epoch_counter = 0; > > connector->tile_blob_ptr = NULL; > > connector->status = connector_status_unknown; > > connector->display_info.panel_orientation = > > diff --git a/drivers/gpu/drm/drm_probe_helper.c > > b/drivers/gpu/drm/drm_probe_helper.c > > index 351cbc40f0f8..5131ae56e676 100644 > > --- a/drivers/gpu/drm/drm_probe_helper.c > > +++ b/drivers/gpu/drm/drm_probe_helper.c > > @@ -777,6 +777,7 @@ bool drm_helper_hpd_irq_event(struct drm_device > > *dev) > > struct drm_connector_list_iter conn_iter; > > enum drm_connector_status old_status; > > bool changed = false; > > + uint64_t old_epoch_counter; > > > > if (!dev->mode_config.poll_enabled) > > return false; > > @@ -790,20 +791,44 @@ bool drm_helper_hpd_irq_event(struct > > drm_device *dev) > > > > old_status = connector->status; > > > > + old_epoch_counter = connector->epoch_counter; > > + > > + DRM_DEBUG_KMS("[CONNECTOR:%d:%s] Old change counter > > %llu\n", connector->base.id, > > + connector->name, > > + old_epoch_counter); > > + > > connector->status = drm_helper_probe_detect(connector, > > NULL, false); > > Pass a u32 *changed as argument to drm_helper_probe_detect? would > require signature change, > > DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %s > > to %s\n", > > connector->base.id, > > connector->name, > > drm_get_connector_status_name(old_status) > > , > > drm_get_connector_status_name(connector- > > >status)); > > - if (old_status != connector->status) > > + > > + DRM_DEBUG_KMS("[CONNECTOR:%d:%s] New change counter > > %llu\n", > > + connector->base.id, > > + connector->name, > > + connector->epoch_counter); > > + > > + if (old_status != connector->status) { > > changed = true; > > + } > > + > > + /* Check changing of edid when a connector status still > > remains > > +* as "connector_status_connected". > > +*/ > > + if (connector->status == connector_status_connected && > > + old_status == connector_status_connected && > > + old_epoch_counter != connector->epoch_counter) { > > + changed = true; > > + } > > Could we bump the epoch counter for any event being sent out? Would > make more sense. I was thinking about this. That would be quite sane approach I think. But then we might have to change also many other places where it checks connection status only. I will take a look as it anyways looks better than checking both epoch and connection status at the same time. > > } > > drm_connector_list_iter_end(&conn_iter); > > mutex_unlock(&dev->mode_config.mutex); > > > > - if (changed) > > + if (changed) { > > drm_kms_helper_hotplug_event(dev); > > + DRM_DEBUG_KMS("Sent hotplug event\n"); > > + } > > > > return changed; > > } > > diff --git a/include/drm/drm_connector.h > > b/include/drm/drm_connector.h > > index 681cb590f952..a3cc7d0927d6 100644 > > --- a/include/drm/drm_connector.h > > +++ b/include/drm/drm_connector.h > > @@ -1288,6 +1288,9 @@ struct drm_connector { > > /** @override_edid: has the EDID be
Re: [PATCH v3 14/14] drm: rcar-du: Force CMM enablement when resuming
Hi Jacopo, On Thu, Sep 05, 2019 at 12:58:09PM +0200, Jacopo Mondi wrote: > On Tue, Aug 27, 2019 at 03:05:17AM +0300, Laurent Pinchart wrote: > > Hi Jacopo, > > > > (Question for Daniel below) > > > > Thank you for the patch. > > > > On Sun, Aug 25, 2019 at 03:51:54PM +0200, Jacopo Mondi wrote: > >> When resuming from system suspend, the DU driver is responsible for > >> reprogramming and enabling the CMM unit if it was in use at the time > >> the system entered the suspend state. > >> > >> Force the color_mgmt_changed flag to true if any of the DRM color > >> transformation properties was set in the CRTC state duplicated at > >> suspend time, as the CMM gets reprogrammed only if said flag is active in > >> the rcar_du_atomic_commit_update_cmm() method. > >> > >> Signed-off-by: Jacopo Mondi > >> --- > >> drivers/gpu/drm/rcar-du/rcar_du_drv.c | 21 + > >> 1 file changed, 21 insertions(+) > >> > >> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c > >> b/drivers/gpu/drm/rcar-du/rcar_du_drv.c > >> index 018480a8f35c..6e38495fb78f 100644 > >> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c > >> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c > >> @@ -17,6 +17,7 @@ > >> #include > >> #include > >> > >> +#include > >> #include > >> #include > >> #include > >> @@ -482,6 +483,26 @@ static int rcar_du_pm_suspend(struct device *dev) > >> static int rcar_du_pm_resume(struct device *dev) > >> { > >>struct rcar_du_device *rcdu = dev_get_drvdata(dev); > >> + struct drm_atomic_state *state = rcdu->ddev->mode_config.suspend_state; > >> + unsigned int i; > >> + > >> + for (i = 0; i < rcdu->num_crtcs; ++i) { > >> + struct drm_crtc *crtc = &rcdu->crtcs[i].crtc; > >> + struct drm_crtc_state *crtc_state; > >> + > >> + crtc_state = drm_atomic_get_existing_crtc_state(state, crtc); > >> + if (!crtc_state) > >> + continue; > > > > Shouldn't you get the new state here ? > > I have followed the drm_atomic_helper_suspend() call stack, that calls > drm_atomic_helper_duplicate_state() which then assign the crtct state > with drm_atomic_get_crtc_state(), where I read: > > crtc_state = drm_atomic_get_existing_crtc_state(state, crtc); > ... > state->crtcs[index].state = crtc_state; > state->crtcs[index].old_state = crtc->state; > state->crtcs[index].new_state = crtc_state; > > So state or new_state for the purpose of getting back the crtc state > are the same if I'm not mistaken. It seems to be the case, but the documentation of drm_atomic_get_existing_crtc_state() states * This function is deprecated, @drm_atomic_get_old_crtc_state or * @drm_atomic_get_new_crtc_state should be used instead. I would thus use drm_atomic_get_new_crtc_state(). > >> + > >> + /* > >> + * Force re-enablement of CMM after system resume if any > >> + * of the DRM color transformation properties was set in > >> + * the state saved at system suspend time. > >> + */ > >> + if (crtc_state->gamma_lut || crtc_state->degamma_lut || > >> + crtc_state->ctm) > > > > We don't support degamma_lut or crm, so I would drop those. > > yeah, I added them as it was less code to change when we'll support > them. But for now they could be removed. > > > With these small issues addressed, > > > > Reviewed-by: Laurent Pinchart > > > > Shouldn't we however squash this with the previous patch to avoid > > bisection issues ? > > Which one in your opinion? > "drm: rcar-du: kms: Update CMM in atomic commit tail" ? > It seems to me they do quite different things though.. Yes, but suspend/resume will be broken after 13/14 without 14/14. Not the end of the world, but not really nice if we need to bisect suspend/resume issues. > >> + crtc_state->color_mgmt_changed = true; > > > > Daniel, is this something that would make sense in the KMS core (or > > helpers) ? > > > >> + } > >> > >>return drm_mode_config_helper_resume(rcdu->ddev); > >> } -- Regards, Laurent Pinchart
Re: [PATCH v3 01/14] dt-bindings: display: renesas,cmm: Add R-Car CMM documentation
Hi Jacopo, On Fri, Aug 30, 2019 at 08:01:09PM +0200, Jacopo Mondi wrote: > On Mon, Aug 26, 2019 at 01:15:50PM +0300, Laurent Pinchart wrote: > > On Mon, Aug 26, 2019 at 09:59:43AM +0200, Jacopo Mondi wrote: > >> On Mon, Aug 26, 2019 at 09:34:41AM +0200, Geert Uytterhoeven wrote: > >>> On Sun, Aug 25, 2019 at 3:50 PM Jacopo Mondi > >>> wrote: > Add device tree bindings documentation for the Renesas R-Car Display > Unit Color Management Module. > > CMM is the image enhancement module available on each R-Car DU video > channel on R-Car Gen2 and Gen3 SoCs (V3H and V3M excluded). > > Signed-off-by: Jacopo Mondi > >>> > >>> Thanks for your patch! > >>> > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/renesas,cmm.txt > @@ -0,0 +1,33 @@ > +* Renesas R-Car Color Management Module (CMM) > + > +Renesas R-Car image enhancement module connected to R-Car DU video > channels. > + > +Required properties: > + - compatible: shall be one or more of the following: > + - "renesas,cmm-r8a7795": for R8A7795 (R-Car H3) compatible CMM. > + - "renesas,cmm-r8a7796": for R8A7796 (R-Car M3-W) compatible CMM. > + - "renesas,cmm-r8a77965": for R8A77965 (R-Car M3-N) compatible CMM. > + - "renesas,cmm-r8a77990": for R8A77990 (R-Car E3) compatible CMM. > + - "renesas,cmm-r8a77995": for R8A77995 (R-Car D3) compatible CMM. > >>> > >>> Please use "renesas,-cmm" instead of "renesas,cmm-". > >> > >> I actually copied it from the r-car gpio bindings, and I liked > >> cmm- better. If you prefer I can change it though. > >> > + - "renesas,rcar-gen3-cmm": for a generic R-Car Gen3 compatible CMM. > + - "renesas,rcar-gen2-cmm": for a generic R-Car Gen2 compatible CMM. > + > + When the generic compatible string is specified, the SoC-specific > + version corresponding to the platform should be listed first. > + > + - reg: the address base and length of the memory area where CMM control > + registers are mapped to. > + > + - clocks: phandle and clock-specifier pair to the CMM functional clock > + supplier. > >>> > >>> Thinking about yaml validation: > >>> > >>> power-domains? > >>> resets? > >> > >> They should indeed be documented. > > > > How about converting this binding to yaml alreay ? It should be fairly > > simple. > > I'm trying to, and I'm having my portion of fun time at it. > > The definition of the schema itself seems good, but I wonder, is this > the first renesas schema we have? Because it seems to me the schema > validator is having an hard time to digest the examplea 'clocks' and > 'power-domains' properties, which have 1 phandle and 2 specifiers and 1 > phandle and 1 specifier respectively for Rensas SoCs. > > In other words, if in the example I have: > > examples: >- | > cmm0: cmm@fea4 { > compatible = "renesas,r8a7796-cmm"; > reg = <0 0xfea4 0 0x1000>; > clocks = <&cpg 711> < 1 phandle + 1 specifier > resets = <&cpg 711>; > power-domains = <&sysc>; < 1 phandle > }; > > The schema validation is good. > > While if I use an actual example >- | > cmm0: cmm@fea4 { > compatible = "renesas,r8a7796-cmm"; > reg = <0 0xfea4 0 0x1000>; > clocks = <&cpg CPG_MOD 711> < 1 phandle + 2 specifier > resets = <&cpg 711>; > power-domains = <&sysc R8A7796_PD_ALWAYS_ON>; < 1 phandle > }; + 1 specfier > > The schema validation fails... > Error: > Documentation/devicetree/bindings/display/renesas,cmm.example.dts:20.29-30 > syntax error > FATAL ERROR: Unable to parse input tree > > Are clocks properties with > 2 entries and power-domains properties with > > 1 entries supported? > > Because from what I read here: > https://github.com/robherring/yaml-bindings/blob/master/schemas/clock/clock.yaml > "The length of a clock specifier is defined by the value of a #clock-cells > property in the clock provider node." > > And that's expected, but is the examples actually validated against the > clock provider pointed by the phandle? Because in that case, if we had a > yaml schema for the cpg-mssr provider, it would indeed specify clock-cells=2. > > Do we need a schema for cpg-mssr first, or am I doing something else > wrong? I think you just need to #include the headers that define CPG_MOD and R8A7796_PD_ALWAYS_ON. > +Example: > + > + > + cmm0: cmm@fea4 { > + compatible = "renesas,cmm-r8a7796"; > + reg = <0 0xfea4 0 0x1000>; > + power-domains = <&sysc R8A7796_PD_ALWAYS_ON>; > + clocks = <&cpg CPG_MOD 711>; > + resets = <&cpg 711>; > + }; -- Regards, Laurent Pinc
[Bug 111481] AMD Navi GPU frequent freezes on both Manjaro/Ubuntu with kernel 5.3 and mesa 19.2 -git/llvm9
https://bugs.freedesktop.org/show_bug.cgi?id=111481 --- Comment #30 from Mathieu Belanger --- I will disable the workaround friday after work. Then I will report when it will crash. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/vmwgfx: Fix double free in vmw_recv_msg()
On Thu, 2019-08-15 at 09:38 +0100, Colin Ian King wrote: > On 15/08/2019 09:30, Dan Carpenter wrote: > > We recently added a kfree() after the end of the loop: > > > > if (retries == RETRIES) { > > kfree(reply); > > return -EINVAL; > > } > > > > There are two problems. First the test is wrong and because > > retries > > equals RETRIES if we succeed on the last iteration through the > > loop. > > Second if we fail on the last iteration through the loop then the > > kfree > > is a double free. > > > > When you're reading this code, please note the break statement at > > the > > end of the while loop. This patch changes the loop so that if it's > > not > > successful then "reply" is NULL and we can test for that afterward. > > > > Fixes: 6b7c3b86f0b6 ("drm/vmwgfx: fix memory leak when too many > > retries have occurred") > > Signed-off-by: Dan Carpenter > > --- > > drivers/gpu/drm/vmwgfx/vmwgfx_msg.c | 8 +++- > > 1 file changed, 3 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c > > b/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c > > index 59e9d05ab928..0af048d1a815 100644 > > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c > > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c > > @@ -353,7 +353,7 @@ static int vmw_recv_msg(struct rpc_channel > > *channel, void **msg, > > !!(HIGH_WORD(ecx) & > > MESSAGE_STATUS_HB)); > > if ((HIGH_WORD(ebx) & MESSAGE_STATUS_SUCCESS) == 0) { > > kfree(reply); > > - > > + reply = NULL; > > if ((HIGH_WORD(ebx) & MESSAGE_STATUS_CPT) != 0) > > { > > /* A checkpoint occurred. Retry. */ > > continue; > > @@ -377,7 +377,7 @@ static int vmw_recv_msg(struct rpc_channel > > *channel, void **msg, > > > > if ((HIGH_WORD(ecx) & MESSAGE_STATUS_SUCCESS) == 0) { > > kfree(reply); > > - > > + reply = NULL; > > if ((HIGH_WORD(ecx) & MESSAGE_STATUS_CPT) != 0) > > { > > /* A checkpoint occurred. Retry. */ > > continue; > > @@ -389,10 +389,8 @@ static int vmw_recv_msg(struct rpc_channel > > *channel, void **msg, > > break; > > } > > > > - if (retries == RETRIES) { > > - kfree(reply); > > + if (!reply) > > return -EINVAL; > > - } > > > > *msg_len = reply_len; > > *msg = reply; > > > > Dan, Thanks for fixing up my mistake. Thanks, Dan. Sorry for the late reply. Reviewed-by: Thomas Hellström Will push this to fixes. /Thomas ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 01/14] dt-bindings: display: renesas,cmm: Add R-Car CMM documentation
Hi Laurent, On Thu, Sep 5, 2019 at 1:50 PM Laurent Pinchart wrote: > On Fri, Aug 30, 2019 at 08:01:09PM +0200, Jacopo Mondi wrote: > > On Mon, Aug 26, 2019 at 01:15:50PM +0300, Laurent Pinchart wrote: > > > How about converting this binding to yaml alreay ? It should be fairly > > > simple. > > > > I'm trying to, and I'm having my portion of fun time at it. > > > > The definition of the schema itself seems good, but I wonder, is this > > the first renesas schema we have? Because it seems to me the schema > > validator is having an hard time to digest the examplea 'clocks' and > > 'power-domains' properties, which have 1 phandle and 2 specifiers and 1 > > phandle and 1 specifier respectively for Rensas SoCs. > > > > In other words, if in the example I have: > > > > examples: > >- | > > cmm0: cmm@fea4 { > > compatible = "renesas,r8a7796-cmm"; > > reg = <0 0xfea4 0 0x1000>; > > clocks = <&cpg 711> < 1 phandle + 1 specifier > > resets = <&cpg 711>; > > power-domains = <&sysc>; < 1 phandle > > }; > > > > The schema validation is good. > > > > While if I use an actual example > >- | > > cmm0: cmm@fea4 { > > compatible = "renesas,r8a7796-cmm"; > > reg = <0 0xfea4 0 0x1000>; > > clocks = <&cpg CPG_MOD 711> < 1 phandle + 2 specifier > > resets = <&cpg 711>; > > power-domains = <&sysc R8A7796_PD_ALWAYS_ON>; < 1 phandle > > }; + 1 specfier > > > > The schema validation fails... > > Error: > > Documentation/devicetree/bindings/display/renesas,cmm.example.dts:20.29-30 > > syntax error > > FATAL ERROR: Unable to parse input tree > > > > Are clocks properties with > 2 entries and power-domains properties with > > > 1 entries supported? > > > > Because from what I read here: > > https://github.com/robherring/yaml-bindings/blob/master/schemas/clock/clock.yaml > > "The length of a clock specifier is defined by the value of a #clock-cells > > property in the clock provider node." > > > > And that's expected, but is the examples actually validated against the > > clock provider pointed by the phandle? Because in that case, if we had a > > yaml schema for the cpg-mssr provider, it would indeed specify > > clock-cells=2. > > > > Do we need a schema for cpg-mssr first, or am I doing something else > > wrong? > > I think you just need to #include the headers that define CPG_MOD and > R8A7796_PD_ALWAYS_ON. If that helps, you might want to replace the symbols in the examples by their actual values (1 resp. 32). And perhaps keep the symbols as comments? clocks = <&cpg 1 /* CPG_MOD */ 711>; power-domains = <&sysc 32 /* R8A7796_PD_ALWAYS_ON */>; Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
[Bug 204683] amdgpu: ring sdma0 timeout
https://bugzilla.kernel.org/show_bug.cgi?id=204683 --- Comment #6 from Matthias Heinz (m...@familie-heinz.name) --- I had to switch to drm-next to do further bisecting and I think 634092b1b9f67bea23a87b77880df5e8012a411a is causing the problem. I might be wrong though. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/panfrost: Prevent race when handling page fault
When handling a GPU page fault addr_to_drm_mm_node() is used to translate the GPU address to a buffer object. However it is possible for the buffer object to be freed after the function has returned resulting in a use-after-free of the BO. Change addr_to_drm_mm_node to return the panfrost_gem_object with an extra reference on it, preventing the BO from being freed until after the page fault has been handled. Signed-off-by: Steven Price --- I've managed to trigger this, generating the following stack trace. Unable to handle kernel NULL pointer dereference at virtual address 0090 pgd = 33a6a181 [0090] *pgd= Internal error: Oops: 5 [#1] SMP ARM Modules linked in: CPU: 0 PID: 81 Comm: irq/60-mmu Not tainted 5.3.0-rc1+ #4 Hardware name: Rockchip (Device Tree) PC is at panfrost_mmu_map_fault_addr+0x140/0x3a0 LR is at _raw_spin_unlock+0x20/0x3c pc : []lr : []psr: 20030013 sp : ec643e88 ip : ea70ce24 fp : ec5fe840 r10: 0001 r9 : r8 : 000178c9 r7 : r6 : 178c9f00 r5 : ec5fe940 r4 : ea70ce08 r3 : r2 : r1 : ec640e00 r0 : ec5fe940 Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none Control: 10c5387d Table: 29f8406a DAC: 0051 Process irq/60-mmu (pid: 81, stack limit = 0x4b907106) Stack: (0xec643e88 to 0xec644000) 3e80: ec640e00 c07cede0 c0c0b200 ec640e00 3ea0: 000178c9 c0c0b200 c07cede0 eef91040 ec5fe840 0001 3ec0: 00010001 010003c3 00c3 0001 178c9f00 c0508c98 eef91050 3ee0: ec643f34 c07c9188 0001 c0167854 ec643ef8 c0c08448 c07c933c 3f00: ec643f04 fffefffe 3d182b17 ee193468 ee193400 ec5db3c0 ec5db3c0 3f20: c0183840 ec5db3e4 c0cb2874 c0183b08 0001 c018385c e000 ee193400 3f40: ec5db3c0 c0183b8c c0c08448 6013 c01839b8 3d182b17 3f60: e000 ec5d0500 ec5db380 ec642000 ec5db3c0 c0183a64 ed025cd8 3f80: ec5d0538 c0146cfc ec642000 ec5db380 c0146bc0 3fa0: c01010b4 3fc0: 3fe0: 0013 [] (panfrost_mmu_map_fault_addr) from [] (panfrost_mmu_irq_handler_thread+0xf4/0x248) [] (panfrost_mmu_irq_handler_thread) from [] (irq_thread_fn+0x1c/0x58) [] (irq_thread_fn) from [] (irq_thread+0x128/0x240) [] (irq_thread) from [] (kthread+0x13c/0x154) [] (kthread) from [] (ret_from_fork+0x14/0x20) Exception stack(0xec643fb0 to 0xec643ff8) 3fa0: 3fc0: 3fe0: 0013 Code: 1aec eae8 e5143004 e59d2014 (e5933090) ---[ end trace 04eadc3009b8f507 ]--- --- drivers/gpu/drm/panfrost/panfrost_mmu.c | 38 - 1 file changed, 24 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c b/drivers/gpu/drm/panfrost/panfrost_mmu.c index 842bdd7cf6be..115925e7460a 100644 --- a/drivers/gpu/drm/panfrost/panfrost_mmu.c +++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c @@ -392,9 +392,11 @@ void panfrost_mmu_pgtable_free(struct panfrost_file_priv *priv) free_io_pgtable_ops(mmu->pgtbl_ops); } -static struct drm_mm_node *addr_to_drm_mm_node(struct panfrost_device *pfdev, int as, u64 addr) +static struct panfrost_gem_object * +addr_to_drm_mm_node(struct panfrost_device *pfdev, int as, u64 addr) { - struct drm_mm_node *node = NULL; + struct panfrost_gem_object *bo = NULL; + struct drm_mm_node *node; u64 offset = addr >> PAGE_SHIFT; struct panfrost_mmu *mmu; @@ -406,14 +408,17 @@ static struct drm_mm_node *addr_to_drm_mm_node(struct panfrost_device *pfdev, in priv = container_of(mmu, struct panfrost_file_priv, mmu); drm_mm_for_each_node(node, &priv->mm) { - if (offset >= node->start && offset < (node->start + node->size)) + if (offset >= node->start && + offset < (node->start + node->size)) { + bo = drm_mm_node_to_panfrost_bo(node); + drm_gem_object_get(&bo->base.base); goto out; + } } } - out: spin_unlock(&pfdev->as_lock); - return node; + return bo; } #define NUM_FAULT_PAGES (SZ_2M / PAGE_SIZE) @@ -421,29 +426,28 @@ static struct drm_mm_node *addr_to_drm_mm_node(struct panfrost_device *pfdev, in int panfrost_mmu_map_fault_addr(struct panfrost_device *pfdev, int as, u64 addr) { int ret, i; - struct drm_mm_node *node; struct panfrost_gem_object *bo; struct address_space *mapping; pgoff_t page_offset; struct sg_table
Re: [PATCH v2] drm/mcde: Some fixes to handling video mode
On Tue, Sep 3, 2019 at 4:38 PM Stephan Gerhold wrote: > On Tue, Sep 03, 2019 at 11:15:12AM +0200, Linus Walleij wrote: > > /* > >* This is the time to perform LP->HS on D-PHY > >* FIXME: nowhere to get this from: DT property on the DSI? > > + * values like 48 and 72 seen in the vendor code. > >*/ > > - val |= 0 << DSI_VID_DPHY_TIME_REG_WAKEUP_TIME_SHIFT; > > + val |= 48 << DSI_VID_DPHY_TIME_REG_WAKEUP_TIME_SHIFT; > > writel(val, d->regs + DSI_VID_DPHY_TIME); > > I just want to note that the Samsung S3 Mini (golden) seems to use 88 > here. I suppose we will need to see how important this property is, > or if panels can also work with a slightly wrong value. Yeah maybe we should just figure out what to do about this. This is the wakeup time, in clock cycles, for a LP->HS transition on the D-PHY. The panel driver kind of knows knows this timing, so I guess we should add a field to struct mipi_dsi_device such as unsigned int lp_to_hs_wakep, but it needs to come from the device tree since per the manual this value is system dependent: "reg_wakeup_time must be shorter than line duration and depends on the D-PHY cell plus some pipelines delays inserted by the DSI link. This value strongly depends on the PLL programming and as it is a mix of analog and digital timing, it is practically impossible to provide a general formula. By the way, it has to be characterized at system level (validation and/or simulation)." > > - writel(blkeol_pck, d->regs + DSI_VID_VCA_SETTING2); > > + writel((blkeol_pck & > > DSI_VID_VCA_SETTING2_EXACT_BURST_LIMIT_MASK) > > +<< DSI_VID_VCA_SETTING2_EXACT_BURST_LIMIT_SHIFT, > > +d->regs + DSI_VID_VCA_SETTING2); > > It does not make a difference in this case because SHIFT = 0, > but shouldn't you normally shift before applying the mask? > Otherwise, you would wipe out the lower bits before you shift them. OK you're right, I fix it up and resend. Yours, Linus Walleij ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 01/14] dt-bindings: display: renesas,cmm: Add R-Car CMM documentation
Hi Geert, On Thu, Sep 05, 2019 at 02:05:34PM +0200, Geert Uytterhoeven wrote: > On Thu, Sep 5, 2019 at 1:50 PM Laurent Pinchart wrote: > > On Fri, Aug 30, 2019 at 08:01:09PM +0200, Jacopo Mondi wrote: > > > On Mon, Aug 26, 2019 at 01:15:50PM +0300, Laurent Pinchart wrote: > > > > How about converting this binding to yaml alreay ? It should be fairly > > > > simple. > > > > > > I'm trying to, and I'm having my portion of fun time at it. > > > > > > The definition of the schema itself seems good, but I wonder, is this > > > the first renesas schema we have? Because it seems to me the schema > > > validator is having an hard time to digest the examplea 'clocks' and > > > 'power-domains' properties, which have 1 phandle and 2 specifiers and 1 > > > phandle and 1 specifier respectively for Rensas SoCs. > > > > > > In other words, if in the example I have: > > > > > > examples: > > >- | > > > cmm0: cmm@fea4 { > > > compatible = "renesas,r8a7796-cmm"; > > > reg = <0 0xfea4 0 0x1000>; > > > clocks = <&cpg 711> < 1 phandle + 1 specifier > > > resets = <&cpg 711>; > > > power-domains = <&sysc>; < 1 phandle > > > }; > > > > > > The schema validation is good. > > > > > > While if I use an actual example > > >- | > > > cmm0: cmm@fea4 { > > > compatible = "renesas,r8a7796-cmm"; > > > reg = <0 0xfea4 0 0x1000>; > > > clocks = <&cpg CPG_MOD 711> < 1 phandle + 2 > > > specifier > > > resets = <&cpg 711>; > > > power-domains = <&sysc R8A7796_PD_ALWAYS_ON>; < 1 phandle > > > }; + 1 specfier > > > > > > The schema validation fails... > > > Error: > > > Documentation/devicetree/bindings/display/renesas,cmm.example.dts:20.29-30 > > > syntax error > > > FATAL ERROR: Unable to parse input tree > > > > > > Are clocks properties with > 2 entries and power-domains properties with > > > > 1 entries supported? > > > > > > Because from what I read here: > > > https://github.com/robherring/yaml-bindings/blob/master/schemas/clock/clock.yaml > > > "The length of a clock specifier is defined by the value of a #clock-cells > > > property in the clock provider node." > > > > > > And that's expected, but is the examples actually validated against the > > > clock provider pointed by the phandle? Because in that case, if we had a > > > yaml schema for the cpg-mssr provider, it would indeed specify > > > clock-cells=2. > > > > > > Do we need a schema for cpg-mssr first, or am I doing something else > > > wrong? > > > > I think you just need to #include the headers that define CPG_MOD and > > R8A7796_PD_ALWAYS_ON. > > If that helps, you might want to replace the symbols in the examples by their > actual values (1 resp. 32). > > And perhaps keep the symbols as comments? > > clocks = <&cpg 1 /* CPG_MOD */ 711>; > power-domains = <&sysc 32 /* R8A7796_PD_ALWAYS_ON */>; I think adding the required #include at the beginning of the example is a better solution. -- Regards, Laurent Pinchart
[PATCH 01/15] drm/edid: Add drm_hdmi_avi_infoframe_bars()
From: Ville Syrjälä Add a function to fill the AVI infoframe bar information from the standard tv margin properties. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/drm_edid.c | 17 + include/drm/drm_edid.h | 4 2 files changed, 21 insertions(+) diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 82a4ceed3fcf..1e16ee20cd31 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -5282,6 +5282,23 @@ drm_hdmi_avi_infoframe_quant_range(struct hdmi_avi_infoframe *frame, } EXPORT_SYMBOL(drm_hdmi_avi_infoframe_quant_range); +/** + * drm_hdmi_avi_infoframe_bars() - fill the HDMI AVI infoframe + * bar information + * @frame: HDMI AVI infoframe + * @conn_state: connector state + */ +void +drm_hdmi_avi_infoframe_bars(struct hdmi_avi_infoframe *frame, + const struct drm_connector_state *conn_state) +{ + frame->right_bar = conn_state->tv.margins.right; + frame->left_bar = conn_state->tv.margins.left; + frame->top_bar = conn_state->tv.margins.top; + frame->bottom_bar = conn_state->tv.margins.bottom; +} +EXPORT_SYMBOL(drm_hdmi_avi_infoframe_bars); + static enum hdmi_3d_structure s3d_structure_from_display_mode(const struct drm_display_mode *mode) { diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h index b9719418c3d2..e0701b3d3194 100644 --- a/include/drm/drm_edid.h +++ b/include/drm/drm_edid.h @@ -367,6 +367,10 @@ void drm_hdmi_avi_infoframe_colorspace(struct hdmi_avi_infoframe *frame, const struct drm_connector_state *conn_state); +void +drm_hdmi_avi_infoframe_bars(struct hdmi_avi_infoframe *frame, + const struct drm_connector_state *conn_state); + void drm_hdmi_avi_infoframe_quant_range(struct hdmi_avi_infoframe *frame, struct drm_connector *connector, -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/exynos: Move static keyword to the front of declaration
Move the static keyword to the front of declaration of modes, and resolve the following compiler warning that can be seen when building with warnings enabled (W=1): drivers/gpu/drm/exynos/exynos_mixer.c:1074:2: warning: ‘static’ is not at beginning of declaration [-Wold-style-declaration] Signed-off-by: Krzysztof Wilczynski --- Related: https://lore.kernel.org/r/20190827233017.gk9...@google.com drivers/gpu/drm/exynos/exynos_mixer.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c index 7b24338fad3c..6cfdb95fef2f 100644 --- a/drivers/gpu/drm/exynos/exynos_mixer.c +++ b/drivers/gpu/drm/exynos/exynos_mixer.c @@ -1069,9 +1069,9 @@ static bool mixer_mode_fixup(struct exynos_drm_crtc *crtc, struct mixer_context *ctx = crtc->ctx; int width = mode->hdisplay, height = mode->vdisplay, i; - struct { + static const struct { int hdisplay, vdisplay, htotal, vtotal, scan_val; - } static const modes[] = { + } modes[] = { { 720, 480, 858, 525, MXR_CFG_SCAN_NTSC | MXR_CFG_SCAN_SD }, { 720, 576, 864, 625, MXR_CFG_SCAN_PAL | MXR_CFG_SCAN_SD }, { 1280, 720, 1650, 750, MXR_CFG_SCAN_HD_720 | MXR_CFG_SCAN_HD }, -- 2.22.1
[GIT PULL] etnaviv-next-fixes for 5.4
Hi Daniel, hi Dave, a single fix for a error path issue in the newly introduced per-process address space code. Regards, Lucas The following changes since commit 578d2342ec702e5fb8a77983fabb3754ae3e9660: Merge tag 'drm-next-5.4-2019-08-23' of git://people.freedesktop.org/~agd5f/linux into drm-next (2019-08-27 17:22:15 +1000) are available in the Git repository at: https://git.pengutronix.de/git/lst/linux etnaviv/next for you to fetch changes up to dbcc574a4bfad810ce6e118f3cf327c7b6e7c897: drm/etnaviv: fix missing unlock on error in etnaviv_iommuv1_context_alloc() (2019-09-02 12:48:55 +0200) Wei Yongjun (1): drm/etnaviv: fix missing unlock on error in etnaviv_iommuv1_context_alloc() drivers/gpu/drm/etnaviv/etnaviv_iommu.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/3] drm/atomic: Rename crtc_state->pageflip_flags to async_flip
On Wed, Sep 4, 2019 at 2:57 PM Kazlauskas, Nicholas wrote: > > On 2019-09-03 3:06 p.m., Daniel Vetter wrote: > > It's the only flag anyone actually cares about. Plus if we're unlucky, > > the atomic ioctl might need a different flag for async flips. So > > better to abstract this away from the uapi a bit. > > > > Cc: Maarten Lankhorst > > Cc: Michel Dänzer > > Cc: Alex Deucher > > Cc: Adam Jackson > > Cc: Sean Paul > > Cc: David Airlie > > Signed-off-by: Daniel Vetter > > Cc: Maxime Ripard > > Cc: Daniel Vetter > > Cc: Nicholas Kazlauskas > > Cc: Leo Li > > Cc: Harry Wentland > > Cc: David Francis > > Cc: Mario Kleiner > > Cc: Bhawanpreet Lakha > > Cc: Ben Skeggs > > Cc: "Christian König" > > Cc: Ilia Mirkin > > Cc: Sam Ravnborg > > Cc: Chris Wilson > > --- > > Series is: > > Reviewed-by: Nicholas Kazlauskas > > I would like to see a new flag eventually show up for atomic as well, > but the existing one is effectively broken at this point and I would > hope that no userspace is setting it expecting that it actually does > something. You mean it is generally broken? My software uses non-vsync'ed flips for diagnostic purpose and iirc some gpu + driver combo didn't work as expected anymore. But i thought that was one specific driver bug (maybe on AMD + DC)? > > At this point we don't really gain anything from enabling atomic in DDX > I think, most drivers already make use of DRM helpers to map these > legacy IOCTLs to atomic anyway. > One thing i wanted to try, once i hopefully find some time in late 2019 / early 2020 (if nobody else starts working on such a thing earlier), would be to add the ability to pass in a target flip time to the pageflip ioctl for use with VRR. For that i thought adding a new pageflip flag a la DRM_MODE_PAGE_FLIP_TARGETTIME) would be a good way to reuse the existing page_flip_target ioctl and redefine the "uint32 sequence" field of struct drm_mode_crtc_page_flip_target to pass in the target time - or at least the lower 32 bits of a target time. So that would be one more page flip flag for the future. I'd like this to be workable from X11, and the current DDX don't use the atomic interface, apart from the modesetting DDX where it just got disabled by default in xserver master due to various unresolved bugs afaik? thanks, -mario > Nicholas Kazlauskas > > > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 5 ++--- > > drivers/gpu/drm/drm_atomic_helper.c | 2 +- > > drivers/gpu/drm/drm_atomic_state_helper.c | 2 +- > > drivers/gpu/drm/nouveau/dispnv50/wndw.c | 4 ++-- > > include/drm/drm_crtc.h| 8 > > 5 files changed, 10 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > > index 0a71ed1e7762..2f0ef0820f00 100644 > > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > > @@ -5756,8 +5756,7 @@ static void amdgpu_dm_commit_planes(struct > > drm_atomic_state *state, > >* change FB pitch, DCC state, rotation or mirroing. > >*/ > > bundle->flip_addrs[planes_count].flip_immediate = > > - (crtc->state->pageflip_flags & > > - DRM_MODE_PAGE_FLIP_ASYNC) != 0 && > > + crtc->state->async_flip && > > acrtc_state->update_type == UPDATE_TYPE_FAST; > > > > timestamp_ns = ktime_get_ns(); > > @@ -6334,7 +6333,7 @@ static void amdgpu_dm_atomic_commit_tail(struct > > drm_atomic_state *state) > > amdgpu_dm_enable_crtc_interrupts(dev, state, true); > > > > for_each_new_crtc_in_state(state, crtc, new_crtc_state, j) > > - if (new_crtc_state->pageflip_flags & DRM_MODE_PAGE_FLIP_ASYNC) > > + if (new_crtc_state->async_flip) > > wait_for_vblank = false; > > > > /* update planes when needed per crtc*/ > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c > > b/drivers/gpu/drm/drm_atomic_helper.c > > index e9c6112e7f73..1e5293eb66e3 100644 > > --- a/drivers/gpu/drm/drm_atomic_helper.c > > +++ b/drivers/gpu/drm/drm_atomic_helper.c > > @@ -3263,7 +3263,7 @@ static int page_flip_common(struct drm_atomic_state > > *state, > > return PTR_ERR(crtc_state); > > > > crtc_state->event = event; > > - crtc_state->pageflip_flags = flags; > > + crtc_state->async_flip = flags & DRM_MODE_PAGE_FLIP_ASYNC; > > > > plane_state = drm_atomic_get_plane_state(state, plane); > > if (IS_ERR(plane_state)) > > diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c > > b/drivers/gpu/drm/drm_atomic_state_helper.c > > index 46dc264a248b..d0a937fb0c56 100644 > > --- a/drivers/gpu/drm/drm_atomic_state_helper.c > > +++ b/drivers/gpu/drm/drm_atomic_state_helper.c > > @@ -128,7 +128,7 @@ void __drm_atomic_helper_crtc_duplicate_s
Re: Xorg indefinitely hangs in kernelspace
On 05.09.19 10:14, Gerd Hoffmann wrote: > On Tue, Aug 06, 2019 at 09:00:10PM +0300, Jaak Ristioja wrote: >> Hello! >> >> I'm writing to report a crash in the QXL / DRM code in the Linux kernel. >> I originally filed the issue on LaunchPad and more details can be found >> there, although I doubt whether these details are useful. > > Any change with kernel 5.3-rc7 ? Didn't try. Did you change something? I could try, but I've done so before and every time this bug manifests itself with MAJOR.MINOR-rc# I get asked to try version MAJOR.(MINOR+1)-rc# so I guess I could as well give up? Alright, I'll install 5.3-rc7, but once more it might take some time for this bug to expose itself. One thing to note though, is that this occurred much more often with older kernels (or older versions of the Kubuntu desktop/Firefox in my VM), sometimes even several times in an hour of use. Best regards, J
Re: [PATCH] drm/panfrost: Fix regulator_get_optional() misuse
On Thu, Sep 05, 2019 at 10:37:53AM +0100, Steven Price wrote: > Ah, I didn't realise that regulator_get() will return a dummy regulator > if none is provided in the DT. In theory that seems like a nicer > solution to my two commits. However there's still a problem - the dummy > regulator returned from regulator_get() reports errors when > regulator_set_voltage() is called. So I get errors like this: > [ 299.861165] panfrost e82c.mali: Cannot set voltage 110 uV > [ 299.867294] devfreq devfreq0: dvfs failed with (-22) error > (And therefore the frequency isn't being changed) > Ideally we want a dummy regulator that will silently ignore any > regulator_set_voltage() calls. Is that safe? You can't rely on being able to change voltages even if there's a physical regulator available, system constraints or the results of sharing the regulator with other users may prevent changes. I guess at the minute the code is assuming that if you can't vary the regulator it's fixed at the maximum voltage and that it's safe to run at a lower clock with a higher voltage (some devices don't like doing that). If the device always starts up at full speed I guess that's OK. It's certainly in general a bad idea to do this in general, we can't tell how important it is to the consumer that they actually get the voltage that they asked for - for some applications like this it's just adding to the power saving it's likely fine but for others it might break things. If you're happy to change the frequency without the ability to vary the voltage you can query what's supported through the API (the simplest interface is regulator_is_supported_voltage()). You should do the regulator API queries at initialization time since they can be a bit expensive, the usual pattern would be to go through your OPP table and disable states where you can't support the voltage but you *could* also flag states where you just don't set the voltage. That seems especially reasonable if no voltages in the range the device supports can be set. I do note that the current code requires exactly specified voltages with no variation which doesn't match the behaviour you say you're OK with here, what you're describing sounds like the driver should be specifying a voltage range from the hardware specified maximum down to whatever the minimum the OPP supports rather than exactly the OPP voltage. As things are you might also run into voltages that can't be hit exactly (eg, in the Exynos 5433 case in mainline a regulator that only offers steps of 2mV will error out trying to set several of the OPPs). signature.asc Description: PGP signature
Re: [PATCH] drm/vgem: Added page prefaulting
Thank you for the review Andrzej. I'll update the patch to v2 shortly, it should cover all your comments. FYI, I'll be on holiday until September 16 so I might not be able to respond in the following days. Regards, Szymon On 03.09.2019 15:19, Andrzej Hajda wrote: > +CC: $(./script/get_maintainers.pl drivers/gpu/drm/vgem/vgem_drv.c) > > > On 20.08.2019 08:58, Szymon Andrzejuk wrote: >> Page fault handler inside vgem driver now preallocates pages in advance >> when fault occurs for the first time. Pages can be allocated in >> direction of increasing/decreasing addresses, depending on memory access >> profile. In case of random access no preallocation occurs. >> >> Synthetic benchmark showed over 8x bandwidth increase when copying data >> from mmapped vgem buffer with memcpy and ~160 times when accessing mapped >> buffer sequentially. Compiled with gcc 8.2.0 with -O2 flag. >> Unigine Heaven running on custom virgl vtest virtual GPU with vgem buffers >> sees ~17% FPS increase. >> >> This performance increase only occurs when accessing vgem buffer mapped >> using DRM_IOCTL_MODE_MAP_DUMB ioctl. When accessing buffer imported >> from prime fd the vgem page fault handler is not invoked. It's advised >> to use vector streaming copy instructions and avoid sequential accesses >> in this case. Streaming copy brings the performance to be on par with >> similar buffer allocated with memfd_create(2) syscall. >> >> Signed-off-by: Szymon Andrzejuk >> --- >> drivers/gpu/drm/vgem/vgem_drv.c | 177 ++-- >> 1 file changed, 143 insertions(+), 34 deletions(-) >> >> diff --git a/drivers/gpu/drm/vgem/vgem_drv.c >> b/drivers/gpu/drm/vgem/vgem_drv.c >> index 11a8f99ba18c..739ba841e89c 100644 >> --- a/drivers/gpu/drm/vgem/vgem_drv.c >> +++ b/drivers/gpu/drm/vgem/vgem_drv.c >> @@ -34,6 +34,7 @@ >> #include >> #include >> #include >> +#include >> #include "vgem_drv.h" >> >> #define DRIVER_NAME "vgem" >> @@ -50,8 +51,21 @@ static struct vgem_device { >> static void vgem_gem_free_object(struct drm_gem_object *obj) >> { >> struct drm_vgem_gem_object *vgem_obj = to_vgem_bo(obj); >> +int i; >> + >> +mutex_lock(&vgem_obj->pages_lock); >> +if (vgem_obj->pages) { >> +int num_pages = obj->size >> PAGE_SHIFT; >> + >> +for (i = 0; i < num_pages; i++) { >> +if (vgem_obj->pages[i]) >> +put_page(vgem_obj->pages[i]); >> +} >> +kvfree(vgem_obj->pages); >> +vgem_obj->pages = NULL; >> +} >> +mutex_unlock(&vgem_obj->pages_lock); >> >> -kvfree(vgem_obj->pages); >> mutex_destroy(&vgem_obj->pages_lock); >> >> if (obj->import_attach) >> @@ -61,6 +75,72 @@ static void vgem_gem_free_object(struct drm_gem_object >> *obj) >> kfree(vgem_obj); >> } >> >> +static int __vgem_alloc_page(struct page *page, struct vm_area_struct *vma, >> + unsigned long vaddr, >> int page_num) > > unused page_num > > >> +{ >> +unsigned long pfn; >> +int insert_ret; >> + >> +pfn = page_to_pfn(page); >> +insert_ret = vmf_insert_mixed(vma, vaddr, __pfn_to_pfn_t(pfn, PFN_DEV)); >> + >> +if (insert_ret & VM_FAULT_ERROR) >> +return VM_FAULT_ERROR; > > Is it OK to return mask? instead of insert_ret. > > >> + >> +return 0; >> +} >> + >> +static int __vgem_read_mapping_page(struct drm_vgem_gem_object *obj, >> +int >> page_num, struct page **page) >> +{ >> +int ret; >> +struct page *mapped_page; >> + >> +mapped_page = >> shmem_read_mapping_page(file_inode(obj->base.filp)->i_mapping, >> + >> page_num); >> +if (IS_ERR(page)) { >> +switch (PTR_ERR(page)) { >> +case -ENOSPC: >> +case -ENOMEM: >> +ret = VM_FAULT_OOM; >> +break; >> +case -EBUSY: >> +ret = VM_FAULT_RETRY; >> +break; >> +case -EFAULT: >> +case -EINVAL: >> +ret = VM_FAULT_SIGBUS; >> +break; >> +default: >> +WARN_ON(PTR_ERR(page)); >> +ret = VM_FAULT_SIGBUS; >> +break; >> +} >> + >> +return ret; >> +} >> + >> +*page = mapped_page; >> +return 0; >> +} >> + >> +static int __vgem_prepare_single_page(struct drm_vgem_gem_object *obj, >> + >> struct vm_area_struct *vma, >> + int >> page_num, unsigned long vaddr) >> +{ >> +int ret; >> + >> +ret = __vgem_read_mapping_page(obj, page_num, &obj->pages[page_num]); >> +if (ret) >> +return ret;
[git pull] vmwgfx-fixes-5.3
From: Thomas Hellström Dave, Daniel A single fix from Dan for a previous fix that generated a regression. The following changes since commit 6b7c3b86f0b63134b2ab56508921a0853ffa687a: drm/vmwgfx: fix memory leak when too many retries have occurred (2019-08-08 11:22:54 +0200) are available in the Git repository at: git://people.freedesktop.org/~thomash/linux vmwgfx-fixes-5.3 for you to fetch changes up to 08b0c891605acf727e43e3e03a25857d3e789b61: drm/vmwgfx: Fix double free in vmw_recv_msg() (2019-09-05 14:44:28 +0200) Dan Carpenter (1): drm/vmwgfx: Fix double free in vmw_recv_msg() drivers/gpu/drm/vmwgfx/vmwgfx_msg.c | 8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/panfrost: Fix regulator_get_optional() misuse
On 05/09/2019 13:40, Mark Brown wrote: > On Thu, Sep 05, 2019 at 10:37:53AM +0100, Steven Price wrote: > >> Ah, I didn't realise that regulator_get() will return a dummy regulator >> if none is provided in the DT. In theory that seems like a nicer >> solution to my two commits. However there's still a problem - the dummy >> regulator returned from regulator_get() reports errors when >> regulator_set_voltage() is called. So I get errors like this: > >> [ 299.861165] panfrost e82c.mali: Cannot set voltage 110 uV >> [ 299.867294] devfreq devfreq0: dvfs failed with (-22) error > >> (And therefore the frequency isn't being changed) > >> Ideally we want a dummy regulator that will silently ignore any >> regulator_set_voltage() calls. > > Is that safe? You can't rely on being able to change voltages even if > there's a physical regulator available, system constraints or the > results of sharing the regulator with other users may prevent changes. Perhaps I didn't express myself clearly. I mean that in the case of the Hikey960 it would be convenient to have a "dummy regulator" that simply accepted any change because ultimately Linux doesn't have direct control of the voltages but simply requests a particular operating frequency via the mailbox. > I guess at the minute the code is assuming that if you can't vary the > regulator it's fixed at the maximum voltage and that it's safe to run at > a lower clock with a higher voltage (some devices don't like doing that). No - at the moment if the regulator reports an error then the function bails out and doesn't change the frequency. > If the device always starts up at full speed I guess that's OK. It's > certainly in general a bad idea to do this in general, we can't tell how > important it is to the consumer that they actually get the voltage that > they asked for - for some applications like this it's just adding to the > power saving it's likely fine but for others it might break things. Agreed. > If you're happy to change the frequency without the ability to vary the > voltage you can query what's supported through the API (the simplest > interface is regulator_is_supported_voltage()). You should do the > regulator API queries at initialization time since they can be a bit > expensive, the usual pattern would be to go through your OPP table and > disable states where you can't support the voltage but you *could* also > flag states where you just don't set the voltage. That seems especially > reasonable if no voltages in the range the device supports can be set. > > I do note that the current code requires exactly specified voltages with > no variation which doesn't match the behaviour you say you're OK with > here, what you're describing sounds like the driver should be specifying > a voltage range from the hardware specified maximum down to whatever the > minimum the OPP supports rather than exactly the OPP voltage. As things > are you might also run into voltages that can't be hit exactly (eg, in > the Exynos 5433 case in mainline a regulator that only offers steps of > 2mV will error out trying to set several of the OPPs). Perhaps there's a better way of doing devfreq? Panfrost itself doesn't really care must about this - we just need to be able to scaling up/down the operating point depending on load. On many platforms to set the frequency it's necessary to do the dance to set an appropriate voltage before/afterwards, but on the Hikey960 because this is handled via a mailbox we don't actually have a regulator to set the voltage on. My commit[1] supports this by simply not listing the regulator in the DT and assuming that nothing is needed when switching frequency. I'm happy for some other way of handling this if there's a better method. At the moment your change from devm_regulator_get_optional() to devm_regulator_get() is a regression on this platform because it means there is now a dummy regulator which will always fail the regulator_set_voltage() calls preventing frequency changes. And I can't see anything I can do in the DT to fix that. Steve [1] e21dd290881b drm/panfrost: Enable devfreq to work without regulator (plus bug fix in 52282163dfa6) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2] drm/vgem: Added page prefaulting
Page fault handler inside vgem driver now preallocates pages in advance when fault occurs for the first time. Pages can be allocated in direction of increasing/decreasing addresses, depending on memory access profile. In case of random access no preallocation occurs. Synthetic benchmark showed over 8x bandwidth increase when copying data from mmapped vgem buffer with memcpy and ~160 times when accessing mapped buffer sequentially. Compiled with gcc 8.2.0 with -O2 flag. Unigine Heaven running on custom virgl vtest virtual GPU with vgem buffers sees ~17% FPS increase. This performance increase only occurs when accessing vgem buffer mapped using DRM_IOCTL_MODE_MAP_DUMB ioctl. When accessing buffer imported from prime fd the vgem page fault handler is not invoked. It's advised to use vector streaming copy instructions and avoid sequential accesses in this case. Streaming copy brings the performance to be on par with similar buffer allocated with memfd_create(2) syscall. Changes in v2: - Refactored some code based on Andrzej Hajda review - changed return types of __vgem_alloc_page, __vgem_read_mapping_page and __vgem_prepare_single_page from int to vm_fault_t Signed-off-by: Szymon Andrzejuk --- drivers/gpu/drm/vgem/vgem_drv.c | 178 ++-- 1 file changed, 144 insertions(+), 34 deletions(-) diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c index 11a8f99ba18c..155a7911cfc8 100644 --- a/drivers/gpu/drm/vgem/vgem_drv.c +++ b/drivers/gpu/drm/vgem/vgem_drv.c @@ -34,6 +34,7 @@ #include #include #include +#include #include "vgem_drv.h" #define DRIVER_NAME"vgem" @@ -50,8 +51,21 @@ static struct vgem_device { static void vgem_gem_free_object(struct drm_gem_object *obj) { struct drm_vgem_gem_object *vgem_obj = to_vgem_bo(obj); + int i; + + mutex_lock(&vgem_obj->pages_lock); + if (vgem_obj->pages) { + int num_pages = obj->size >> PAGE_SHIFT; + + for (i = 0; i < num_pages; i++) { + if (vgem_obj->pages[i]) + put_page(vgem_obj->pages[i]); + } + kvfree(vgem_obj->pages); + vgem_obj->pages = NULL; + } + mutex_unlock(&vgem_obj->pages_lock); - kvfree(vgem_obj->pages); mutex_destroy(&vgem_obj->pages_lock); if (obj->import_attach) @@ -61,6 +75,68 @@ static void vgem_gem_free_object(struct drm_gem_object *obj) kfree(vgem_obj); } +static vm_fault_t __vgem_alloc_page(struct page *page, struct vm_area_struct *vma, + unsigned long vaddr) +{ + unsigned long pfn; + vm_fault_t insert_ret; + + pfn = page_to_pfn(page); + insert_ret = vmf_insert_mixed(vma, vaddr, __pfn_to_pfn_t(pfn, PFN_DEV)); + + if (insert_ret & VM_FAULT_ERROR) + return insert_ret; + + return 0; +} + +static vm_fault_t __vgem_read_mapping_page(struct drm_vgem_gem_object *obj, + int page_num, struct page **page) +{ + vm_fault_t ret; + struct page *mapped_page; + + mapped_page = shmem_read_mapping_page(file_inode(obj->base.filp)->i_mapping, + page_num); + if (IS_ERR(page)) { + switch (PTR_ERR(page)) { + case -ENOSPC: + case -ENOMEM: + ret = VM_FAULT_OOM; + break; + case -EBUSY: + ret = VM_FAULT_RETRY; + break; + case -EFAULT: + case -EINVAL: + ret = VM_FAULT_SIGBUS; + break; + default: + WARN_ON(PTR_ERR(page)); + ret = VM_FAULT_SIGBUS; + break; + } + + return ret; + } + + *page = mapped_page; + return 0; +} + +static vm_fault_t __vgem_prepare_single_page(struct drm_vgem_gem_object *obj, + struct vm_area_struct *vma, + int page_num, unsigned long vaddr) +{ + vm_fault_t ret; + + ret = __vgem_read_mapping_page(obj, page_num, &obj->pages[page_num]); + if (ret) + return ret; + + return __vgem_alloc_page(obj->pages[page_num], vma, vaddr); +} + static vm_fault_t vgem_gem_fault(struct vm_fault *vmf) { struct vm_area_struct *vma = vmf->vma; @@ -70,6 +146,8 @@ static vm_fault_t vgem_gem_fault(struct vm_fault *vmf) vm_fault_t ret = VM_FAULT_SIGBUS; loff_t num_pages; pgoff_t page_offset; + int page_num, page_prep_re
Re: [PATCH v3 08/14] drm: rcar-du: Add support for CMM
Hi Laurent, On Thu, Sep 05, 2019 at 02:17:12PM +0300, Laurent Pinchart wrote: > Hi Jacopo, > > > +/** > > + * rcar_cmm_enable() - enable the CMM unit > > + * > > + * @pdev: The platform device associated with the CMM instance > > + * > > + * Enable the CMM unit by enabling the parent clock and enabling the > > CMM > > + * components, such as 1-D LUT, if requested. > > + */ > > +int rcar_cmm_enable(struct platform_device *pdev) > > +{ > > + struct rcar_cmm *rcmm = platform_get_drvdata(pdev); > > + int ret; > > + > > + if (!rcmm) > > + return -EPROBE_DEFER; > > >>> > > >>> This function is called in rcar_du_crtc_atomic_enable(), so that's not > > >>> the right error code. It seems we need another function for the CMM API > > >>> to defer probing :-/ I would call it rcar_cmm_init(). This check would > > >>> then be removed. > > >> > > >> I agree about the return code, but not the name, as this function > > >> actually enables the CMM. > > > > > > I meant creating a new rcar_cmm_init() function that would just have the > > > !rcmm check. > > > > > >> PROBE_DEFER does not make any sense here, I > > >> wonder where it come from, as the probing of CMM and DU has long > > >> happened once we get here (at least, I assume so, if we receive a > > >> gamma_table, userspace has already been running, and both DU and CMM > > >> should have probed. Otherwise, we can exploit the newly created device > > >> link, and make sure DU probes after the CMM). > > >> > > >> I would just change the return value here, and possibly use the device > > >> link to ensure the correct probing sequence. > > > > > > How does device link help here ? > > > > Currently it doesn't, as we are creating a stateless link. > > > > But if we go for a managed device link (which is the default, by the > > way, you have to opt-out from it) we can guarantee the CMM has probed > > before the DU probes, so that we have a guarantee when we get here > > !rcmm cannot happen. > > > > https://www.kernel.org/doc/html/v5.2-rc7/driver-api/device_link.html > > "The consumer devices are not probed before the supplier is bound to a > > driver, > > and they’re unbound before the supplier is unbound." > > > > As we create the link, the CMM is the supplier of DU, so we could just > > drop the DL_FLAG_STATELESS flag in device_link_add() in 10/14. > > > > Does this match your understanding ? > > Except there's a bit of a chicken and egg issue, as you call > device_link_add() from rcar_du_cmm_init(), which thus require the DU > driver to probe first :-) For this to work we would probably need an > early initcall in the DU driver. > Yes indeed, the point where the link is created at the moment is too late... Is it worth an early initcall, or should we just assume that at the point where the LUT is operated userspace has already been running and both the CMM and the DU have probed already? signature.asc Description: PGP signature
Re: linux-next: Tree for Sep 4 (amd/display/)
On 2019-09-04 4:58 p.m., Randy Dunlap wrote: > On 9/4/19 6:34 AM, Stephen Rothwell wrote: >> Hi all, >> >> News: this will be the last linux-next I will release until Sept 30. >> >> Changes since 20190903: >> > > on x86_64: > > In file included from > ../drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn20/display_rq_dlg_calc_20v2.c:77:0: > ../drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn20/../dml_inline_defs.h: > In function ‘dml_min’: > ../drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn20/../dml_inline_defs.h:34:1: > error: SSE register return with SSE disabled > Is that fixed by Stephen's fixup here? https://lists.freedesktop.org/archives/dri-devel/2019-September/234292.html Thanks, Harry > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 01/14] dt-bindings: display: renesas,cmm: Add R-Car CMM documentation
Hi Laurent, Geert, On Thu, Sep 05, 2019 at 03:20:59PM +0300, Laurent Pinchart wrote: > Hi Geert, > > On Thu, Sep 05, 2019 at 02:05:34PM +0200, Geert Uytterhoeven wrote: > > On Thu, Sep 5, 2019 at 1:50 PM Laurent Pinchart wrote: > > > On Fri, Aug 30, 2019 at 08:01:09PM +0200, Jacopo Mondi wrote: > > > > On Mon, Aug 26, 2019 at 01:15:50PM +0300, Laurent Pinchart wrote: > > > > > How about converting this binding to yaml alreay ? It should be fairly > > > > > simple. > > > > > > > > I'm trying to, and I'm having my portion of fun time at it. > > > > > > > > The definition of the schema itself seems good, but I wonder, is this > > > > the first renesas schema we have? Because it seems to me the schema > > > > validator is having an hard time to digest the examplea 'clocks' and > > > > 'power-domains' properties, which have 1 phandle and 2 specifiers and 1 > > > > phandle and 1 specifier respectively for Rensas SoCs. > > > > > > > > In other words, if in the example I have: > > > > > > > > examples: > > > >- | > > > > cmm0: cmm@fea4 { > > > > compatible = "renesas,r8a7796-cmm"; > > > > reg = <0 0xfea4 0 0x1000>; > > > > clocks = <&cpg 711> < 1 phandle + 1 specifier > > > > resets = <&cpg 711>; > > > > power-domains = <&sysc>; < 1 phandle > > > > }; > > > > > > > > The schema validation is good. > > > > > > > > While if I use an actual example > > > >- | > > > > cmm0: cmm@fea4 { > > > > compatible = "renesas,r8a7796-cmm"; > > > > reg = <0 0xfea4 0 0x1000>; > > > > clocks = <&cpg CPG_MOD 711> < 1 phandle + 2 > > > > specifier > > > > resets = <&cpg 711>; > > > > power-domains = <&sysc R8A7796_PD_ALWAYS_ON>; < 1 phandle > > > > }; + 1 > > > > specfier > > > > > > > > The schema validation fails... > > > > Error: > > > > Documentation/devicetree/bindings/display/renesas,cmm.example.dts:20.29-30 > > > > syntax error > > > > FATAL ERROR: Unable to parse input tree > > > > > > > > Are clocks properties with > 2 entries and power-domains properties with > > > > > 1 entries supported? > > > > > > > > Because from what I read here: > > > > https://github.com/robherring/yaml-bindings/blob/master/schemas/clock/clock.yaml > > > > "The length of a clock specifier is defined by the value of a > > > > #clock-cells > > > > property in the clock provider node." > > > > > > > > And that's expected, but is the examples actually validated against the > > > > clock provider pointed by the phandle? Because in that case, if we had a > > > > yaml schema for the cpg-mssr provider, it would indeed specify > > > > clock-cells=2. > > > > > > > > Do we need a schema for cpg-mssr first, or am I doing something else > > > > wrong? > > > > > > I think you just need to #include the headers that define CPG_MOD and > > > R8A7796_PD_ALWAYS_ON. > > > > If that helps, you might want to replace the symbols in the examples by > > their > > actual values (1 resp. 32). > > > > And perhaps keep the symbols as comments? > > > > clocks = <&cpg 1 /* CPG_MOD */ 711>; > > power-domains = <&sysc 32 /* R8A7796_PD_ALWAYS_ON */>; > > I think adding the required #include at the beginning of the example is > a better solution. I didn't realize that, but it actually makes sense, as the example is extracted and actually compiled from the yaml schema.. brilliant! With a simple: --- a/Documentation/devicetree/bindings/display/renesas,cmm.yaml +++ b/Documentation/devicetree/bindings/display/renesas,cmm.yaml @@ -51,6 +51,9 @@ additionalProperties: false examples: - | +#include +#include The example now compiles. Thanks, I will submit the bindings in yaml format in next iteration. > > -- > Regards, > > Laurent Pinchart signature.asc Description: PGP signature
Re: [PATCH v3 08/14] drm: rcar-du: Add support for CMM
Hi Jacopo, On Thu, Sep 05, 2019 at 03:14:53PM +0200, Jacopo Mondi wrote: > On Thu, Sep 05, 2019 at 02:17:12PM +0300, Laurent Pinchart wrote: > > Hi Jacopo, > > > >> +/** > >> + * rcar_cmm_enable() - enable the CMM unit > >> + * > >> + * @pdev: The platform device associated with the CMM instance > >> + * > >> + * Enable the CMM unit by enabling the parent clock and enabling the > >> CMM > >> + * components, such as 1-D LUT, if requested. > >> + */ > >> +int rcar_cmm_enable(struct platform_device *pdev) > >> +{ > >> + struct rcar_cmm *rcmm = platform_get_drvdata(pdev); > >> + int ret; > >> + > >> + if (!rcmm) > >> + return -EPROBE_DEFER; > > > > This function is called in rcar_du_crtc_atomic_enable(), so that's not > > the right error code. It seems we need another function for the CMM API > > to defer probing :-/ I would call it rcar_cmm_init(). This check would > > then be removed. > > I agree about the return code, but not the name, as this function > actually enables the CMM. > >>> > >>> I meant creating a new rcar_cmm_init() function that would just have the > >>> !rcmm check. > >>> > PROBE_DEFER does not make any sense here, I > wonder where it come from, as the probing of CMM and DU has long > happened once we get here (at least, I assume so, if we receive a > gamma_table, userspace has already been running, and both DU and CMM > should have probed. Otherwise, we can exploit the newly created device > link, and make sure DU probes after the CMM). > > I would just change the return value here, and possibly use the device > link to ensure the correct probing sequence. > >>> > >>> How does device link help here ? > >> > >> Currently it doesn't, as we are creating a stateless link. > >> > >> But if we go for a managed device link (which is the default, by the > >> way, you have to opt-out from it) we can guarantee the CMM has probed > >> before the DU probes, so that we have a guarantee when we get here > >> !rcmm cannot happen. > >> > >> https://www.kernel.org/doc/html/v5.2-rc7/driver-api/device_link.html > >> "The consumer devices are not probed before the supplier is bound to a > >> driver, > >> and they’re unbound before the supplier is unbound." > >> > >> As we create the link, the CMM is the supplier of DU, so we could just > >> drop the DL_FLAG_STATELESS flag in device_link_add() in 10/14. > >> > >> Does this match your understanding ? > > > > Except there's a bit of a chicken and egg issue, as you call > > device_link_add() from rcar_du_cmm_init(), which thus require the DU > > driver to probe first :-) For this to work we would probably need an > > early initcall in the DU driver. > > Yes indeed, the point where the link is created at the moment is too > late... Is it worth an early initcall, or should we just assume that > at the point where the LUT is operated userspace has already been > running and both the CMM and the DU have probed already? We should at least guard against crashes, that's why I've proposed an init function in the CMM driver for the sole purpose of making sure the device has been probed, and deferring probe of the DU. -- Regards, Laurent Pinchart
Re: [PATCH 4/7] drm/omap: Implement CTM property for CRTC using OVL managers CPR matrix
On 05/09/2019 13:05, Laurent Pinchart wrote: > Hi Jyri, > > On Thu, Sep 05, 2019 at 01:00:51PM +0300, Jyri Sarha wrote: >> On 05/09/2019 00:52, Laurent Pinchart wrote: static void omap_crtc_write_crtc_properties(struct drm_crtc *crtc) { struct omap_drm_private *priv = crtc->dev->dev_private; @@ -402,7 +428,16 @@ static void omap_crtc_write_crtc_properties(struct drm_crtc *crtc) info.default_color = 0x00; info.trans_enabled = false; info.partial_alpha_enabled = false; - info.cpr_enable = false; + + if (crtc->state->ctm) { + struct drm_color_ctm *ctm = + (struct drm_color_ctm *) crtc->state->ctm->data; + + info.cpr_enable = true; + omap_crtc_cpr_coefs_from_ctm(ctm, &info.cpr_coefs); >>> >>> As an optimisation it would be nice to only write the coefficients when >>> they actually change. That could be implemented on top of this series. >> >> E.g. apply this ? >> >> - if (crtc->state->ctm) >> + if (crtc->state->color_mgmt_changed && crtc->state->ctm) > > Something like that, but .mgr_setup() should then be taught not to write > unchanged CTM tables to registers. Do you think it would be worth it ? Hmmm, jess I should do it like this: if (crtc->state->color_mgmt_changed) { if (crtc->state->ctm) { ... + } else { + info.cpr_enable = false; + } } This way the whole CPR functionality is turned off, if the there is no CTM in the crtc state. >>> >>> Yes, but you would also need to update .mgr_setup() :-) A new >>> color_mgmt_changed flag would be needed in the info structure too. >> >> I am starting to thing that such an "optimization" may not be worth the >> added complexity. The arithmetic and writing three registers is not that >> costly and we do not commit a new crtc state that often. > > We call omap_crtc_write_crtc_properties() in omap_crtc_atomic_flush(), > so that's at every page flip... > Still, the mgr_setup() accesses five different registers even if we do not touch CPR settings (another 4 registers). All of those have static values in the mainline omapdrm (there are custom properties to control those in ti-linux). I would rather keep this patch as it is and implement another one with a cached struct omap_overlay_manager_info, that calls mgr_setup() only if the info values have changed. With the cached values in place the unneeded conversion arithmetic can also be skipped based on color_mgmt_changed. BR, Jyri -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/panfrost: Fix regulator_get_optional() misuse
On 05/09/2019 15:02, Steven Price wrote: > On 05/09/2019 13:40, Mark Brown wrote: >> On Thu, Sep 05, 2019 at 10:37:53AM +0100, Steven Price wrote: >> >>> Ah, I didn't realise that regulator_get() will return a dummy regulator >>> if none is provided in the DT. In theory that seems like a nicer >>> solution to my two commits. However there's still a problem - the dummy >>> regulator returned from regulator_get() reports errors when >>> regulator_set_voltage() is called. So I get errors like this: >> >>> [ 299.861165] panfrost e82c.mali: Cannot set voltage 110 uV >>> [ 299.867294] devfreq devfreq0: dvfs failed with (-22) error >> >>> (And therefore the frequency isn't being changed) >> >>> Ideally we want a dummy regulator that will silently ignore any >>> regulator_set_voltage() calls. >> >> Is that safe? You can't rely on being able to change voltages even if >> there's a physical regulator available, system constraints or the >> results of sharing the regulator with other users may prevent changes. > > Perhaps I didn't express myself clearly. I mean that in the case of the > Hikey960 it would be convenient to have a "dummy regulator" that simply > accepted any change because ultimately Linux doesn't have direct control > of the voltages but simply requests a particular operating frequency via > the mailbox. > >> I guess at the minute the code is assuming that if you can't vary the >> regulator it's fixed at the maximum voltage and that it's safe to run at >> a lower clock with a higher voltage (some devices don't like doing that). > > No - at the moment if the regulator reports an error then the function > bails out and doesn't change the frequency. > >> If the device always starts up at full speed I guess that's OK. It's >> certainly in general a bad idea to do this in general, we can't tell how >> important it is to the consumer that they actually get the voltage that >> they asked for - for some applications like this it's just adding to the >> power saving it's likely fine but for others it might break things. > > Agreed. > >> If you're happy to change the frequency without the ability to vary the >> voltage you can query what's supported through the API (the simplest >> interface is regulator_is_supported_voltage()). You should do the >> regulator API queries at initialization time since they can be a bit >> expensive, the usual pattern would be to go through your OPP table and >> disable states where you can't support the voltage but you *could* also >> flag states where you just don't set the voltage. That seems especially >> reasonable if no voltages in the range the device supports can be set. >> >> I do note that the current code requires exactly specified voltages with >> no variation which doesn't match the behaviour you say you're OK with >> here, what you're describing sounds like the driver should be specifying >> a voltage range from the hardware specified maximum down to whatever the >> minimum the OPP supports rather than exactly the OPP voltage. As things >> are you might also run into voltages that can't be hit exactly (eg, in >> the Exynos 5433 case in mainline a regulator that only offers steps of >> 2mV will error out trying to set several of the OPPs). In case we need a fixed voltage, simply add a new fixed-regulator in the board/soc DT, use this regulator for panfrost and use the same fixed voltage in the OPP table. It relies on a correct DT, but isn't is the goal of mainline linux ? Neil > > Perhaps there's a better way of doing devfreq? Panfrost itself doesn't > really care must about this - we just need to be able to scaling up/down > the operating point depending on load. > > On many platforms to set the frequency it's necessary to do the dance to > set an appropriate voltage before/afterwards, but on the Hikey960 > because this is handled via a mailbox we don't actually have a regulator > to set the voltage on. My commit[1] supports this by simply not listing > the regulator in the DT and assuming that nothing is needed when > switching frequency. I'm happy for some other way of handling this if > there's a better method. > > At the moment your change from devm_regulator_get_optional() to > devm_regulator_get() is a regression on this platform because it means > there is now a dummy regulator which will always fail the > regulator_set_voltage() calls preventing frequency changes. And I can't > see anything I can do in the DT to fix that. > > Steve > > [1] e21dd290881b drm/panfrost: Enable devfreq to work without regulator > (plus bug fix in 52282163dfa6) > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/3] drm/atomic: Rename crtc_state->pageflip_flags to async_flip
On Thu, Sep 5, 2019 at 2:33 PM Mario Kleiner wrote: > On Wed, Sep 4, 2019 at 2:57 PM Kazlauskas, Nicholas > wrote: > > > > On 2019-09-03 3:06 p.m., Daniel Vetter wrote: > > > It's the only flag anyone actually cares about. Plus if we're unlucky, > > > the atomic ioctl might need a different flag for async flips. So > > > better to abstract this away from the uapi a bit. > > > > > > Cc: Maarten Lankhorst > > > Cc: Michel Dänzer > > > Cc: Alex Deucher > > > Cc: Adam Jackson > > > Cc: Sean Paul > > > Cc: David Airlie > > > Signed-off-by: Daniel Vetter > > > Cc: Maxime Ripard > > > Cc: Daniel Vetter > > > Cc: Nicholas Kazlauskas > > > Cc: Leo Li > > > Cc: Harry Wentland > > > Cc: David Francis > > > Cc: Mario Kleiner > > > Cc: Bhawanpreet Lakha > > > Cc: Ben Skeggs > > > Cc: "Christian König" > > > Cc: Ilia Mirkin > > > Cc: Sam Ravnborg > > > Cc: Chris Wilson > > > --- > > > > Series is: > > > > Reviewed-by: Nicholas Kazlauskas > > > > I would like to see a new flag eventually show up for atomic as well, > > but the existing one is effectively broken at this point and I would > > hope that no userspace is setting it expecting that it actually does > > something. > > You mean it is generally broken? My software uses non-vsync'ed flips > for diagnostic purpose and iirc some gpu + driver combo didn't work as > expected anymore. But i thought that was one specific driver bug > (maybe on AMD + DC)? atomic ioctl + FLIP_ASYNC does not do anything special. FLIP_ASYNC with the legacy page_flip ioctl works correctly. This is in the core ioctl handler (the flag simply did nothing for atomic), so irrespective of whether the driver has a bug or not with FLIP_ASYNC. > > At this point we don't really gain anything from enabling atomic in DDX > > I think, most drivers already make use of DRM helpers to map these > > legacy IOCTLs to atomic anyway. > > > > One thing i wanted to try, once i hopefully find some time in late > 2019 / early 2020 (if nobody else starts working on such a thing > earlier), would be to add the ability to pass in a target flip time to > the pageflip ioctl for use with VRR. For that i thought adding a new > pageflip flag a la DRM_MODE_PAGE_FLIP_TARGETTIME) would be a good way > to reuse the existing page_flip_target ioctl and redefine the "uint32 > sequence" field of struct drm_mode_crtc_page_flip_target to pass in > the target time - or at least the lower 32 bits of a target time. Yeah target time (instead of just target vblank that we currently have) has been discussed before. It's just that no one yet typed an implementation (driver and userspace and all that). > So that would be one more page flip flag for the future. I'd like this > to be workable from X11, and the current DDX don't use the atomic > interface, apart from the modesetting DDX where it just got disabled > by default in xserver master due to various unresolved bugs afaik? It was a bit too simplistic a conversion. But for specific features (like doing a page flip with atomic instead of the legacy page_flip ioctl because you need that one new feature) you can just use the atomic ioctl - legacy ioctl and atomic can be mixed like that, because underneath it's all atomic anyway. The trouble with the modesetting atomic is that atomic isn't a simple 1:1 translation of the old ioctls, and the implementation that landed was simply broken. And no one has put in the work to make it solid, so it got backed out. -Daniel > > thanks, > -mario > > > Nicholas Kazlauskas > > > > > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 5 ++--- > > > drivers/gpu/drm/drm_atomic_helper.c | 2 +- > > > drivers/gpu/drm/drm_atomic_state_helper.c | 2 +- > > > drivers/gpu/drm/nouveau/dispnv50/wndw.c | 4 ++-- > > > include/drm/drm_crtc.h| 8 > > > 5 files changed, 10 insertions(+), 11 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > > > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > > > index 0a71ed1e7762..2f0ef0820f00 100644 > > > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > > > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > > > @@ -5756,8 +5756,7 @@ static void amdgpu_dm_commit_planes(struct > > > drm_atomic_state *state, > > >* change FB pitch, DCC state, rotation or mirroing. > > >*/ > > > bundle->flip_addrs[planes_count].flip_immediate = > > > - (crtc->state->pageflip_flags & > > > - DRM_MODE_PAGE_FLIP_ASYNC) != 0 && > > > + crtc->state->async_flip && > > > acrtc_state->update_type == UPDATE_TYPE_FAST; > > > > > > timestamp_ns = ktime_get_ns(); > > > @@ -6334,7 +6333,7 @@ static void amdgpu_dm_atomic_commit_tail(struct > > > drm_atomic_state *state) > > > amdgpu_dm_enable_crtc_interrupts(dev, state, true); > > > > > > for_each
RE: [PATCH v4] video: hyperv: hyperv_fb: Obtain screen resolution from Hyper-V host
From: Wei Hu Sent: Thursday, September 5, 2019 2:12 AM > > Beginning from Windows 10 RS5+, VM screen resolution is obtained from host. > The "video=hyperv_fb" boot time option is not needed, but still can be > used to overwrite what the host specifies. The VM resolution on the host > could be set by executing the powershell "set-vmvideo" command. > > Signed-off-by: Iouri Tarassov > Signed-off-by: Wei Hu > --- > v2: > - Implemented fallback when version negotiation failed. > - Defined full size for supported_resolution array. > > v3: > - Corrected the synthvid major and minor version comparison problem. > > v4: > - Changed function name to synthvid_ver_ge(). > > drivers/video/fbdev/hyperv_fb.c | 159 +--- > 1 file changed, 147 insertions(+), 12 deletions(-) > Reviewed-by: Michael Kelley
RE: [PATCH v3] video: hyperv: hyperv_fb: Support deferred IO for Hyper-V frame buffer driver
From: Wei Hu Sent: Thursday, September 5, 2019 1:29 AM > > Without deferred IO support, hyperv_fb driver informs the host to refresh > the entire guest frame buffer at fixed rate, e.g. at 20Hz, no matter there > is screen update or not. This patch supports deferred IO for screens in > graphics mode and also enables the frame buffer on-demand refresh. The > highest refresh rate is still set at 20Hz. > > Currently Hyper-V only takes a physical address from guest as the starting > address of frame buffer. This implies the guest must allocate contiguous > physical memory for frame buffer. In addition, Hyper-V Gen 2 VMs only > accept address from MMIO region as frame buffer address. Due to these > limitations on Hyper-V host, we keep a shadow copy of frame buffer > in the guest. This means one more copy of the dirty rectangle inside > guest when doing the on-demand refresh. This can be optimized in the > future with help from host. For now the host performance gain from deferred > IO outweighs the shadow copy impact in the guest. > > Signed-off-by: Wei Hu > --- > v2: Incorporated review comments from Michael Kelley > - Increased dirty rectangle by one row in deferred IO case when sending > to Hyper-V. > - Corrected the dirty rectangle size in the text mode. > - Added more comments. > - Other minor code cleanups. > > v3: Incorporated more review comments > - Removed a few unnecessary variable tests > > drivers/video/fbdev/Kconfig | 1 + > drivers/video/fbdev/hyperv_fb.c | 216 +--- > 2 files changed, 197 insertions(+), 20 deletions(-) > Reviewed-by: Michael Kelley
Re: [PATCH 1/3] drm/atomic: Take the atomic toys away from X
Op 03-09-2019 om 21:06 schreef Daniel Vetter: > The -modesetting ddx has a totally broken idea of how atomic works: > - doesn't disable old connectors, assuming they get auto-disable like > with the legacy setcrtc > - assumes ASYNC_FLIP is wired through for the atomic ioctl > - not a single call to TEST_ONLY > > Iow the implementation is a 1:1 translation of legacy ioctls to > atomic, which is a) broken b) pointless. > > We already have bugs in both i915 and amdgpu-DC where this prevents us > from enabling neat features. > > If anyone ever cares about atomic in X we can easily add a new atomic > level (req->value == 2) for X to get back the shiny toys. > > Since these broken versions of -modesetting have been shipping, > there's really no other way to get out of this bind. > > References: https://gitlab.freedesktop.org/xorg/xserver/issues/629 > References: https://gitlab.freedesktop.org/xorg/xserver/merge_requests/180 > Cc: Maarten Lankhorst > Cc: Michel Dänzer > Cc: Alex Deucher > Cc: Adam Jackson > Cc: Sean Paul > Cc: David Airlie > Cc: sta...@vger.kernel.org > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/drm_ioctl.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c > index 2c120c58f72d..1cb7b4c3c87c 100644 > --- a/drivers/gpu/drm/drm_ioctl.c > +++ b/drivers/gpu/drm/drm_ioctl.c > @@ -334,6 +334,9 @@ drm_setclientcap(struct drm_device *dev, void *data, > struct drm_file *file_priv) > file_priv->universal_planes = req->value; > break; > case DRM_CLIENT_CAP_ATOMIC: > + /* The modesetting DDX has a totally broken idea of atomic. */ > + if (strstr(current->comm, "X")) > + return -EOPNOTSUPP; > if (!drm_core_check_feature(dev, DRIVER_ATOMIC)) > return -EOPNOTSUPP; > if (req->value > 1) Good riddance! Missing one more: commit abbc0697d5fbf53f74ce0bcbe936670199764cfa Author: Dave Airlie Date: Wed Apr 24 16:33:29 2019 +1000 drm/fb: revert the i915 Actually configure untiled displays from master This code moved in here in master, so revert it the same way. This is the same revert as 9fa246256e09 ("Revert "drm/i915/fbdev: Actually configure untiled displays"") in drm-fixes. Signed-off-by: Dave Airlie Can we unrevert that now? With that fixed, on the whole series: Reviewed-by: Maarten Lankhorst ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 111482] Sapphire Pulse RX 5700 XT power consumption
https://bugs.freedesktop.org/show_bug.cgi?id=111482 --- Comment #12 from Robert --- Andrew, you're my hero ;-) While I'm even more sad now (because I now think that this issue will be indeed never be fixed) I now at least can imagine what's going on. As you recommended I changed resolution to 1920x1080. That's quite common I would say. And tata! Indeed "sensors" reported 8W. Then I changed to 3840x2160. Not so usual I guess but the 2nd biggest resolution my monitor supports. Still 8W! And then back to 5120x1440. And tata! 33W :-( That could mean two things: 1) There is a bug somewhere (firmware, driver, ...) with this resolution which causes that high idle power consumption or 2) which is even worse I suspect that with this resolution the 5700XT acts like you have plugged in two monitors. And from what I read throughout all Navi10 reviews multi monitor setups and power consumption was and still is a problem with AMD graphic cards in general. Oh man, that's something I really didn't calculated with :-( Yeah, in that case I can really only hope for the Intel Xe graphics cards next year (if they really build a consumer card which nobody knows yet ;-) ). I still don't understand why in console where KMS is enabled 8W is enough and while running KDE Plasma in that resolution it takes round about 33W. But maybe it really has something to do with acceleration. It doesn't even consider reducing memory clock at least a little bit even if I do nothing and haven't even started any program. That's the funny thing in general: As long as I don't start Firefox, Thunderbird or something like that "sensors" don't even work. It just prints garbage values and takes minutes to complete. If I start one of the programs mentioned above the screen flickers very shortly and afterwards "sensors" works as expected. I guess only the AMD god knows what that means ;-) At least I now know that I can't do anything further. It just would be cool if one of the AMD engineers could confirm my assumption that with a resolution of 5144x1440 the card always runs at highest memory clock speed as it does with a multi monitor setup (from what I've read so far). -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel