Re: [PATCH 1/2] drm/ttm: fix out-of-bounds read in ttm_put_pages() v2
On 4/8/19 9:13 PM, Christian König wrote: When ttm_put_pages() tries to figure out whether it's dealing with transparent hugepages, it just reads past the bounds of the pages array without a check. v2: simplify the test if enough pages are left in the array (Christian). Series is Reviewed-by: Junwei Zhang Regards, Jerry Signed-off-by: Jann Horn Signed-off-by: Christian König Fixes: 5c42c64f7d54 ("drm/ttm: fix the fix for huge compound pages") Cc: sta...@vger.kernel.org --- drivers/gpu/drm/ttm/ttm_page_alloc.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc.c b/drivers/gpu/drm/ttm/ttm_page_alloc.c index f841accc2c00..f77c81db161b 100644 --- a/drivers/gpu/drm/ttm/ttm_page_alloc.c +++ b/drivers/gpu/drm/ttm/ttm_page_alloc.c @@ -730,7 +730,8 @@ static void ttm_put_pages(struct page **pages, unsigned npages, int flags, } #ifdef CONFIG_TRANSPARENT_HUGEPAGE - if (!(flags & TTM_PAGE_FLAG_DMA32)) { + if (!(flags & TTM_PAGE_FLAG_DMA32) && + (npages - i) >= HPAGE_PMD_NR) { for (j = 0; j < HPAGE_PMD_NR; ++j) if (p++ != pages[i + j]) break; @@ -759,7 +760,7 @@ static void ttm_put_pages(struct page **pages, unsigned npages, int flags, unsigned max_size, n2free; spin_lock_irqsave(&huge->lock, irq_flags); - while (i < npages) { + while ((npages - i) >= HPAGE_PMD_NR) { struct page *p = pages[i]; unsigned j; ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amdgpu: Add a check to avoid panic because of unexpected irqs
IP initialize ras in late_init, because of the BUGs of PSP or any other components, driver receives unexpected irqs. It is ok to add such check anyway. Signed-off-by: xinhui pan --- drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 8 +++- drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 8 +++- drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 8 +++- 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c index 7e749ba6fb4a..54432fa8d420 100644 --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c @@ -4844,10 +4844,16 @@ static int gfx_v9_0_cp_ecc_error_irq(struct amdgpu_device *adev, struct amdgpu_irq_src *source, struct amdgpu_iv_entry *entry) { + struct ras_common_if *ras_if = adev->gfx.ras_if; struct ras_dispatch_if ih_data = { - .head = *adev->gfx.ras_if, .entry = entry, }; + + if (!ras_if) + return 0; + + ih_data.head = *ras_if; + DRM_ERROR("CP ECC ERROR IRQ\n"); amdgpu_ras_interrupt_dispatch(adev, &ih_data); return 0; diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c index dafe91770863..b9c009236a33 100644 --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c @@ -297,10 +297,16 @@ static int gmc_v9_0_process_ecc_irq(struct amdgpu_device *adev, struct amdgpu_irq_src *source, struct amdgpu_iv_entry *entry) { + struct ras_common_if *ras_if = adev->gmc.ras_if; struct ras_dispatch_if ih_data = { - .head = *adev->gmc.ras_if, .entry = entry, }; + + if (!ras_if) + return 0; + + ih_data.head = *ras_if; + amdgpu_ras_interrupt_dispatch(adev, &ih_data); return 0; } diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c index 2a2025f548a1..304d7ba0772c 100644 --- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c @@ -1865,10 +1865,16 @@ static int sdma_v4_0_process_ecc_irq(struct amdgpu_device *adev, struct amdgpu_irq_src *source, struct amdgpu_iv_entry *entry) { + struct ras_common_if *ras_if = adev->sdma.ras_if; struct ras_dispatch_if ih_data = { - .head = *adev->sdma.ras_if, .entry = entry, }; + + if (!ras_if) + return 0; + + ih_data.head = *ras_if; + amdgpu_ras_interrupt_dispatch(adev, &ih_data); return 0; } -- 2.17.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH libdrm 1/8] new syncobj extension v3
v2: drop not implemented IOCTLs and flags v3: add transfer/signal ioctls Signed-off-by: Chunming Zhou Signed-off-by: Christian König --- include/drm/drm.h | 35 +++ 1 file changed, 35 insertions(+) diff --git a/include/drm/drm.h b/include/drm/drm.h index 85c685a2..26f51bca 100644 --- a/include/drm/drm.h +++ b/include/drm/drm.h @@ -729,8 +729,18 @@ struct drm_syncobj_handle { __u32 pad; }; +struct drm_syncobj_transfer { +__u32 src_handle; +__u32 dst_handle; +__u64 src_point; +__u64 dst_point; +__u32 flags; +__u32 pad; +}; + #define DRM_SYNCOBJ_WAIT_FLAGS_WAIT_ALL (1 << 0) #define DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT (1 << 1) +#define DRM_SYNCOBJ_WAIT_FLAGS_WAIT_AVAILABLE (1 << 2) struct drm_syncobj_wait { __u64 handles; /* absolute timeout */ @@ -741,12 +751,31 @@ struct drm_syncobj_wait { __u32 pad; }; +struct drm_syncobj_timeline_wait { +__u64 handles; +/* wait on specific timeline point for every handles*/ +__u64 points; +/* absolute timeout */ +__s64 timeout_nsec; +__u32 count_handles; +__u32 flags; +__u32 first_signaled; /* only valid when not waiting all */ +__u32 pad; +}; + struct drm_syncobj_array { __u64 handles; __u32 count_handles; __u32 pad; }; +struct drm_syncobj_timeline_array { +__u64 handles; +__u64 points; +__u32 count_handles; +__u32 pad; +}; + /* Query current scanout sequence number */ struct drm_crtc_get_sequence { __u32 crtc_id; /* requested crtc_id */ @@ -903,6 +932,12 @@ extern "C" { #define DRM_IOCTL_MODE_GET_LEASE DRM_IOWR(0xC8, struct drm_mode_get_lease) #define DRM_IOCTL_MODE_REVOKE_LEASEDRM_IOWR(0xC9, struct drm_mode_revoke_lease) +#define DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT DRM_IOWR(0xCA, struct drm_syncobj_timeline_wait) +#define DRM_IOCTL_SYNCOBJ_QUERY DRM_IOWR(0xCB, struct drm_syncobj_timeline_array) +#define DRM_IOCTL_SYNCOBJ_TRANSFER DRM_IOWR(0xCC, struct drm_syncobj_transfer) +#define DRM_IOCTL_SYNCOBJ_TIMELINE_SIGNAL DRM_IOWR(0xCD, struct drm_syncobj_timeline_array) + + /** * Device specific ioctls should only be in their respective headers * The device specific ioctl range is from 0x40 to 0x9f. -- 2.17.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH libdrm 6/8] expose timeline signal/export/import interfaces v2
v2: adapt to new one transfer ioctl Signed-off-by: Chunming Zhou --- amdgpu/amdgpu-symbol-check | 3 ++ amdgpu/amdgpu.h| 51 amdgpu/amdgpu_cs.c | 68 ++ 3 files changed, 122 insertions(+) diff --git a/amdgpu/amdgpu-symbol-check b/amdgpu/amdgpu-symbol-check index 67ba3039..0cc54e5e 100755 --- a/amdgpu/amdgpu-symbol-check +++ b/amdgpu/amdgpu-symbol-check @@ -51,10 +51,13 @@ amdgpu_cs_submit amdgpu_cs_submit_raw amdgpu_cs_submit_raw2 amdgpu_cs_syncobj_export_sync_file +amdgpu_cs_syncobj_export_sync_file2 amdgpu_cs_syncobj_import_sync_file +amdgpu_cs_syncobj_import_sync_file2 amdgpu_cs_syncobj_query amdgpu_cs_syncobj_reset amdgpu_cs_syncobj_signal +amdgpu_cs_syncobj_timeline_signal amdgpu_cs_syncobj_timeline_wait amdgpu_cs_syncobj_wait amdgpu_cs_wait_fences diff --git a/amdgpu/amdgpu.h b/amdgpu/amdgpu.h index dcf662e9..b5bd3ed9 100644 --- a/amdgpu/amdgpu.h +++ b/amdgpu/amdgpu.h @@ -1501,6 +1501,23 @@ int amdgpu_cs_syncobj_reset(amdgpu_device_handle dev, int amdgpu_cs_syncobj_signal(amdgpu_device_handle dev, const uint32_t *syncobjs, uint32_t syncobj_count); +/** + * Signal kernel timeline sync objects. + * + * \param dev - \c [in] device handle + * \param syncobjs - \c [in] array of sync object handles + * \param points - \c [in] array of timeline points + * \param syncobj_count - \c [in] number of handles in syncobjs + * + * \return 0 on success\n + * <0 - Negative POSIX Error code + * +*/ +int amdgpu_cs_syncobj_timeline_signal(amdgpu_device_handle dev, + const uint32_t *syncobjs, + uint64_t *points, + uint32_t syncobj_count); + /** * Wait for one or all sync objects to signal. * @@ -1618,7 +1635,41 @@ int amdgpu_cs_syncobj_export_sync_file(amdgpu_device_handle dev, int amdgpu_cs_syncobj_import_sync_file(amdgpu_device_handle dev, uint32_t syncobj, int sync_file_fd); +/** + * Export kernel timeline sync object to a sync_file. + * + * \param dev- \c [in] device handle + * \param syncobj- \c [in] sync object handle + * \param point - \c [in] timeline point + * \param flags - \c [in] flags + * \param sync_file_fd - \c [out] sync_file file descriptor. + * + * \return 0 on success\n + * <0 - Negative POSIX Error code + * + */ +int amdgpu_cs_syncobj_export_sync_file2(amdgpu_device_handle dev, + uint32_t syncobj, + uint64_t point, + uint32_t flags, + int *sync_file_fd); +/** + * Import kernel timeline sync object from a sync_file. + * + * \param dev- \c [in] device handle + * \param syncobj- \c [in] sync object handle + * \param point - \c [in] timeline point + * \param sync_file_fd - \c [in] sync_file file descriptor. + * + * \return 0 on success\n + * <0 - Negative POSIX Error code + * + */ +int amdgpu_cs_syncobj_import_sync_file2(amdgpu_device_handle dev, + uint32_t syncobj, + uint64_t point, + int sync_file_fd); /** * Export an amdgpu fence as a handle (syncobj or fd). * diff --git a/amdgpu/amdgpu_cs.c b/amdgpu/amdgpu_cs.c index b8b0d566..1c02d16f 100644 --- a/amdgpu/amdgpu_cs.c +++ b/amdgpu/amdgpu_cs.c @@ -649,6 +649,18 @@ drm_public int amdgpu_cs_syncobj_signal(amdgpu_device_handle dev, return drmSyncobjSignal(dev->fd, syncobjs, syncobj_count); } +drm_public int amdgpu_cs_syncobj_timeline_signal(amdgpu_device_handle dev, +const uint32_t *syncobjs, +uint64_t *points, +uint32_t syncobj_count) +{ + if (NULL == dev) + return -EINVAL; + + return drmSyncobjTimelineSignal(dev->fd, syncobjs, + points, syncobj_count); +} + drm_public int amdgpu_cs_syncobj_wait(amdgpu_device_handle dev, uint32_t *handles, unsigned num_handles, int64_t timeout_nsec, unsigned flags, @@ -724,6 +736,62 @@ drm_public int amdgpu_cs_syncobj_import_sync_file(amdgpu_device_handle dev, return drmSyncobjImportSyncFile(dev->fd, syncobj, sync_file_fd); } +drm_public int amdgpu_cs_syncobj_export_sync_file2(amdgpu_device_handle dev, + uint32_t syncobj, + uint64_t point, + uint32_t flags, +
[PATCH libdrm 4/8] wrap syncobj timeline query/wait APIs for amdgpu v3
v2: symbos are stored in lexical order. v3: drop export/import and extra query indirection Signed-off-by: Chunming Zhou Signed-off-by: Christian König --- amdgpu/amdgpu-symbol-check | 2 ++ amdgpu/amdgpu.h| 39 ++ amdgpu/amdgpu_cs.c | 23 ++ 3 files changed, 64 insertions(+) diff --git a/amdgpu/amdgpu-symbol-check b/amdgpu/amdgpu-symbol-check index 96a44b40..67ba3039 100755 --- a/amdgpu/amdgpu-symbol-check +++ b/amdgpu/amdgpu-symbol-check @@ -52,8 +52,10 @@ amdgpu_cs_submit_raw amdgpu_cs_submit_raw2 amdgpu_cs_syncobj_export_sync_file amdgpu_cs_syncobj_import_sync_file +amdgpu_cs_syncobj_query amdgpu_cs_syncobj_reset amdgpu_cs_syncobj_signal +amdgpu_cs_syncobj_timeline_wait amdgpu_cs_syncobj_wait amdgpu_cs_wait_fences amdgpu_cs_wait_semaphore diff --git a/amdgpu/amdgpu.h b/amdgpu/amdgpu.h index d6de3b8d..dcf662e9 100644 --- a/amdgpu/amdgpu.h +++ b/amdgpu/amdgpu.h @@ -1521,6 +1521,45 @@ int amdgpu_cs_syncobj_wait(amdgpu_device_handle dev, int64_t timeout_nsec, unsigned flags, uint32_t *first_signaled); +/** + * Wait for one or all sync objects on their points to signal. + * + * \param dev- \c [in] self-explanatory + * \param handles - \c [in] array of sync object handles + * \param points - \c [in] array of sync points to wait + * \param num_handles - \c [in] self-explanatory + * \param timeout_nsec - \c [in] self-explanatory + * \param flags - \c [in] a bitmask of DRM_SYNCOBJ_WAIT_FLAGS_* + * \param first_signaled - \c [in] self-explanatory + * + * \return 0 on success\n + * -ETIME - Timeout + * <0 - Negative POSIX Error code + * + */ +int amdgpu_cs_syncobj_timeline_wait(amdgpu_device_handle dev, + uint32_t *handles, uint64_t *points, + unsigned num_handles, + int64_t timeout_nsec, unsigned flags, + uint32_t *first_signaled); +/** + * Query sync objects payloads. + * + * \param dev- \c [in] self-explanatory + * \param handles - \c [in] array of sync object handles + * \param points - \c [out] array of sync points returned, which presents + * syncobj payload. + * \param num_handles - \c [in] self-explanatory + * + * \return 0 on success\n + * -ETIME - Timeout + * <0 - Negative POSIX Error code + * + */ +int amdgpu_cs_syncobj_query(amdgpu_device_handle dev, + uint32_t *handles, uint64_t *points, + unsigned num_handles); + /** * Export kernel sync object to shareable fd. * diff --git a/amdgpu/amdgpu_cs.c b/amdgpu/amdgpu_cs.c index 5bedf748..b8b0d566 100644 --- a/amdgpu/amdgpu_cs.c +++ b/amdgpu/amdgpu_cs.c @@ -661,6 +661,29 @@ drm_public int amdgpu_cs_syncobj_wait(amdgpu_device_handle dev, flags, first_signaled); } +drm_public int amdgpu_cs_syncobj_timeline_wait(amdgpu_device_handle dev, + uint32_t *handles, uint64_t *points, + unsigned num_handles, + int64_t timeout_nsec, unsigned flags, + uint32_t *first_signaled) +{ + if (NULL == dev) + return -EINVAL; + + return drmSyncobjTimelineWait(dev->fd, handles, points, num_handles, + timeout_nsec, flags, first_signaled); +} + +drm_public int amdgpu_cs_syncobj_query(amdgpu_device_handle dev, + uint32_t *handles, uint64_t *points, + unsigned num_handles) +{ + if (NULL == dev) + return -EINVAL; + + return drmSyncobjQuery(dev->fd, handles, points, num_handles); +} + drm_public int amdgpu_cs_export_syncobj(amdgpu_device_handle dev, uint32_t handle, int *shared_fd) -- 2.17.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH libdrm 8/8] add syncobj timeline tests v3
v2: drop DRM_SYNCOBJ_CREATE_TYPE_TIMELINE, fix timeout calculation, fix some warnings v3: add export/import and cpu signal testing cases Signed-off-by: Chunming Zhou Signed-off-by: Christian König --- tests/amdgpu/Makefile.am | 3 +- tests/amdgpu/amdgpu_test.c | 11 ++ tests/amdgpu/amdgpu_test.h | 21 +++ tests/amdgpu/meson.build | 2 +- tests/amdgpu/syncobj_tests.c | 290 +++ 5 files changed, 325 insertions(+), 2 deletions(-) create mode 100644 tests/amdgpu/syncobj_tests.c diff --git a/tests/amdgpu/Makefile.am b/tests/amdgpu/Makefile.am index 48278848..920882d0 100644 --- a/tests/amdgpu/Makefile.am +++ b/tests/amdgpu/Makefile.am @@ -34,4 +34,5 @@ amdgpu_test_SOURCES = \ uve_ib.h \ deadlock_tests.c \ vm_tests.c \ - ras_tests.c + ras_tests.c \ + syncobj_tests.c diff --git a/tests/amdgpu/amdgpu_test.c b/tests/amdgpu/amdgpu_test.c index 8fc7a0b9..214c7fce 100644 --- a/tests/amdgpu/amdgpu_test.c +++ b/tests/amdgpu/amdgpu_test.c @@ -57,6 +57,7 @@ #define DEADLOCK_TESTS_STR "Deadlock Tests" #define VM_TESTS_STR "VM Tests" #define RAS_TESTS_STR "RAS Tests" +#define SYNCOBJ_TIMELINE_TESTS_STR "SYNCOBJ TIMELINE Tests" /** * Open handles for amdgpu devices @@ -123,6 +124,12 @@ static CU_SuiteInfo suites[] = { .pCleanupFunc = suite_ras_tests_clean, .pTests = ras_tests, }, + { + .pName = SYNCOBJ_TIMELINE_TESTS_STR, + .pInitFunc = suite_syncobj_timeline_tests_init, + .pCleanupFunc = suite_syncobj_timeline_tests_clean, + .pTests = syncobj_timeline_tests, + }, CU_SUITE_INFO_NULL, }; @@ -176,6 +183,10 @@ static Suites_Active_Status suites_active_stat[] = { .pName = RAS_TESTS_STR, .pActive = suite_ras_tests_enable, }, + { + .pName = SYNCOBJ_TIMELINE_TESTS_STR, + .pActive = suite_syncobj_timeline_tests_enable, + }, }; diff --git a/tests/amdgpu/amdgpu_test.h b/tests/amdgpu/amdgpu_test.h index bcd0bc7e..36675ea3 100644 --- a/tests/amdgpu/amdgpu_test.h +++ b/tests/amdgpu/amdgpu_test.h @@ -216,6 +216,27 @@ CU_BOOL suite_ras_tests_enable(void); extern CU_TestInfo ras_tests[]; +/** + * Initialize syncobj timeline test suite + */ +int suite_syncobj_timeline_tests_init(); + +/** + * Deinitialize syncobj timeline test suite + */ +int suite_syncobj_timeline_tests_clean(); + +/** + * Decide if the suite is enabled by default or not. + */ +CU_BOOL suite_syncobj_timeline_tests_enable(void); + +/** + * Tests in syncobj timeline test suite + */ +extern CU_TestInfo syncobj_timeline_tests[]; + + /** * Helper functions */ diff --git a/tests/amdgpu/meson.build b/tests/amdgpu/meson.build index 95ed9305..1726cb43 100644 --- a/tests/amdgpu/meson.build +++ b/tests/amdgpu/meson.build @@ -24,7 +24,7 @@ if dep_cunit.found() files( 'amdgpu_test.c', 'basic_tests.c', 'bo_tests.c', 'cs_tests.c', 'vce_tests.c', 'uvd_enc_tests.c', 'vcn_tests.c', 'deadlock_tests.c', - 'vm_tests.c', 'ras_tests.c', + 'vm_tests.c', 'ras_tests.c', 'syncobj_tests.c', ), dependencies : [dep_cunit, dep_threads], include_directories : [inc_root, inc_drm, include_directories('../../amdgpu')], diff --git a/tests/amdgpu/syncobj_tests.c b/tests/amdgpu/syncobj_tests.c new file mode 100644 index ..a0c627d7 --- /dev/null +++ b/tests/amdgpu/syncobj_tests.c @@ -0,0 +1,290 @@ +/* + * Copyright 2017 Advanced Micro Devices, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR + * OTHER DEALINGS IN THE SOFTWARE. + * +*/ + +#include "CUnit/Basic.h" + +#include "amdgpu_test.h" +#include "amdgpu_drm.h" +#include "amdgpu_internal.h" +#include + +static amdgpu_device_handle device_handle; +static uint32_t major_version; +static uint32_t minor_version; + +static void a
[PATCH libdrm 5/8] add timeline signal/transfer ioctls v2
v2: use one transfer ioctl Signed-off-by: Chunming Zhou --- xf86drm.c | 33 + xf86drm.h | 6 ++ 2 files changed, 39 insertions(+) diff --git a/xf86drm.c b/xf86drm.c index 66e0c985..d57c4218 100644 --- a/xf86drm.c +++ b/xf86drm.c @@ -4280,6 +4280,21 @@ drm_public int drmSyncobjSignal(int fd, const uint32_t *handles, return ret; } +drm_public int drmSyncobjTimelineSignal(int fd, const uint32_t *handles, + uint64_t *points, uint32_t handle_count) +{ +struct drm_syncobj_timeline_array args; +int ret; + +memclear(args); +args.handles = (uintptr_t)handles; +args.points = (uint64_t)(uintptr_t)points; +args.count_handles = handle_count; + +ret = drmIoctl(fd, DRM_IOCTL_SYNCOBJ_TIMELINE_SIGNAL, &args); +return ret; +} + drm_public int drmSyncobjTimelineWait(int fd, uint32_t *handles, uint64_t *points, unsigned num_handles, int64_t timeout_nsec, unsigned flags, @@ -4322,4 +4337,22 @@ drm_public int drmSyncobjQuery(int fd, uint32_t *handles, uint64_t *points, return 0; } +drm_public int drmSyncobjTransfer(int fd, + uint32_t dst_handle, uint64_t dst_point, + uint32_t src_handle, uint64_t src_point, + uint32_t flags) +{ +struct drm_syncobj_transfer args; +int ret; + +memclear(args); +args.src_handle = src_handle; +args.dst_handle = dst_handle; +args.src_point = src_point; +args.dst_point = dst_point; +args.flags = flags; + +ret = drmIoctl(fd, DRM_IOCTL_SYNCOBJ_TRANSFER, &args); +return ret; +} diff --git a/xf86drm.h b/xf86drm.h index 60c7a84f..3fb1d1ca 100644 --- a/xf86drm.h +++ b/xf86drm.h @@ -876,12 +876,18 @@ extern int drmSyncobjWait(int fd, uint32_t *handles, unsigned num_handles, uint32_t *first_signaled); extern int drmSyncobjReset(int fd, const uint32_t *handles, uint32_t handle_count); extern int drmSyncobjSignal(int fd, const uint32_t *handles, uint32_t handle_count); +extern int drmSyncobjTimelineSignal(int fd, const uint32_t *handles, + uint64_t *points, uint32_t handle_count); extern int drmSyncobjTimelineWait(int fd, uint32_t *handles, uint64_t *points, unsigned num_handles, int64_t timeout_nsec, unsigned flags, uint32_t *first_signaled); extern int drmSyncobjQuery(int fd, uint32_t *handles, uint64_t *points, uint32_t handle_count); +extern int drmSyncobjTransfer(int fd, + uint32_t dst_handle, uint64_t dst_point, + uint32_t src_handle, uint64_t src_point, + uint32_t flags); #if defined(__cplusplus) } -- 2.17.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH libdrm 7/8] wrap transfer interfaces
Signed-off-by: Chunming Zhou --- amdgpu/amdgpu.h| 22 ++ amdgpu/amdgpu_cs.c | 16 2 files changed, 38 insertions(+) diff --git a/amdgpu/amdgpu.h b/amdgpu/amdgpu.h index b5bd3ed9..2350835b 100644 --- a/amdgpu/amdgpu.h +++ b/amdgpu/amdgpu.h @@ -1670,6 +1670,28 @@ int amdgpu_cs_syncobj_import_sync_file2(amdgpu_device_handle dev, uint32_t syncobj, uint64_t point, int sync_file_fd); + +/** + * transfer between syncbojs. + * + * \param dev- \c [in] device handle + * \param dst_handle - \c [in] sync object handle + * \param dst_point - \c [in] timeline point, 0 presents dst is binary + * \param src_handle - \c [in] sync object handle + * \param src_point - \c [in] timeline point, 0 presents src is binary + * \param flags - \c [in] flags + * + * \return 0 on success\n + * <0 - Negative POSIX Error code + * + */ +int amdgpu_cs_syncobj_transfer(amdgpu_device_handle dev, + uint32_t dst_handle, + uint64_t dst_point, + uint32_t src_handle, + uint64_t src_point, + uint32_t flags); + /** * Export an amdgpu fence as a handle (syncobj or fd). * diff --git a/amdgpu/amdgpu_cs.c b/amdgpu/amdgpu_cs.c index 1c02d16f..a1c1af55 100644 --- a/amdgpu/amdgpu_cs.c +++ b/amdgpu/amdgpu_cs.c @@ -792,6 +792,22 @@ out: return ret; } +drm_public int amdgpu_cs_syncobj_transfer(amdgpu_device_handle dev, + uint32_t dst_handle, + uint64_t dst_point, + uint32_t src_handle, + uint64_t src_point, + uint32_t flags) +{ + if (NULL == dev) + return -EINVAL; + + return drmSyncobjTransfer(dev->fd, + dst_handle, dst_point, + src_handle, src_point, + flags); +} + drm_public int amdgpu_cs_submit_raw(amdgpu_device_handle dev, amdgpu_context_handle context, amdgpu_bo_list_handle bo_list_handle, -- 2.17.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH libdrm 3/8] add timeline wait/query ioctl v2
v2: drop export/import Signed-off-by: Chunming Zhou --- xf86drm.c | 44 xf86drm.h | 6 ++ 2 files changed, 50 insertions(+) diff --git a/xf86drm.c b/xf86drm.c index 18ad7c58..66e0c985 100644 --- a/xf86drm.c +++ b/xf86drm.c @@ -4279,3 +4279,47 @@ drm_public int drmSyncobjSignal(int fd, const uint32_t *handles, ret = drmIoctl(fd, DRM_IOCTL_SYNCOBJ_SIGNAL, &args); return ret; } + +drm_public int drmSyncobjTimelineWait(int fd, uint32_t *handles, uint64_t *points, + unsigned num_handles, + int64_t timeout_nsec, unsigned flags, + uint32_t *first_signaled) +{ +struct drm_syncobj_timeline_wait args; +int ret; + +memclear(args); +args.handles = (uintptr_t)handles; +args.points = (uint64_t)(uintptr_t)points; +args.timeout_nsec = timeout_nsec; +args.count_handles = num_handles; +args.flags = flags; + +ret = drmIoctl(fd, DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT, &args); +if (ret < 0) +return -errno; + +if (first_signaled) +*first_signaled = args.first_signaled; +return ret; +} + + +drm_public int drmSyncobjQuery(int fd, uint32_t *handles, uint64_t *points, + uint32_t handle_count) +{ +struct drm_syncobj_timeline_array args; +int ret; + +memclear(args); +args.handles = (uintptr_t)handles; +args.points = (uint64_t)(uintptr_t)points; +args.count_handles = handle_count; + +ret = drmIoctl(fd, DRM_IOCTL_SYNCOBJ_QUERY, &args); +if (ret) +return ret; +return 0; +} + + diff --git a/xf86drm.h b/xf86drm.h index 887ecc76..60c7a84f 100644 --- a/xf86drm.h +++ b/xf86drm.h @@ -876,6 +876,12 @@ extern int drmSyncobjWait(int fd, uint32_t *handles, unsigned num_handles, uint32_t *first_signaled); extern int drmSyncobjReset(int fd, const uint32_t *handles, uint32_t handle_count); extern int drmSyncobjSignal(int fd, const uint32_t *handles, uint32_t handle_count); +extern int drmSyncobjTimelineWait(int fd, uint32_t *handles, uint64_t *points, + unsigned num_handles, + int64_t timeout_nsec, unsigned flags, + uint32_t *first_signaled); +extern int drmSyncobjQuery(int fd, uint32_t *handles, uint64_t *points, + uint32_t handle_count); #if defined(__cplusplus) } -- 2.17.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH libdrm 2/8] addr cs chunk for syncobj timeline
Signed-off-by: Chunming Zhou --- include/drm/amdgpu_drm.h | 9 + 1 file changed, 9 insertions(+) diff --git a/include/drm/amdgpu_drm.h b/include/drm/amdgpu_drm.h index e3a97da4..ab53f2e0 100644 --- a/include/drm/amdgpu_drm.h +++ b/include/drm/amdgpu_drm.h @@ -528,6 +528,8 @@ struct drm_amdgpu_gem_va { #define AMDGPU_CHUNK_ID_SYNCOBJ_OUT 0x05 #define AMDGPU_CHUNK_ID_BO_HANDLES 0x06 #define AMDGPU_CHUNK_ID_SCHEDULED_DEPENDENCIES 0x07 +#define AMDGPU_CHUNK_ID_SYNCOBJ_TIMELINE_WAIT0x08 +#define AMDGPU_CHUNK_ID_SYNCOBJ_TIMELINE_SIGNAL 0x09 struct drm_amdgpu_cs_chunk { __u32 chunk_id; @@ -608,6 +610,13 @@ struct drm_amdgpu_cs_chunk_sem { __u32 handle; }; +struct drm_amdgpu_cs_chunk_syncobj { + __u32 handle; + __u32 flags; + __u64 point; +}; + + #define AMDGPU_FENCE_TO_HANDLE_GET_SYNCOBJ 0 #define AMDGPU_FENCE_TO_HANDLE_GET_SYNCOBJ_FD 1 #define AMDGPU_FENCE_TO_HANDLE_GET_SYNC_FILE_FD2 -- 2.17.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
RE: [PATCH] drm/amdgpu: Add a check to avoid panic because of unexpected irqs
Reviewed-by: Evan Quan > -Original Message- > From: Pan, Xinhui > Sent: Tuesday, April 09, 2019 4:59 PM > To: amd-gfx@lists.freedesktop.org > Cc: Deucher, Alexander ; Quan, Evan > > Subject: [PATCH] drm/amdgpu: Add a check to avoid panic because of > unexpected irqs > > IP initialize ras in late_init, because of the BUGs of PSP or any other > components, driver receives unexpected irqs. It is ok to add such check > anyway. > > Signed-off-by: xinhui pan > --- > drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 8 +++- > drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 8 +++- > drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 8 +++- > 3 files changed, 21 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c > b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c > index 7e749ba6fb4a..54432fa8d420 100644 > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c > @@ -4844,10 +4844,16 @@ static int gfx_v9_0_cp_ecc_error_irq(struct > amdgpu_device *adev, > struct amdgpu_irq_src *source, > struct amdgpu_iv_entry *entry) > { > + struct ras_common_if *ras_if = adev->gfx.ras_if; > struct ras_dispatch_if ih_data = { > - .head = *adev->gfx.ras_if, > .entry = entry, > }; > + > + if (!ras_if) > + return 0; > + > + ih_data.head = *ras_if; > + > DRM_ERROR("CP ECC ERROR IRQ\n"); > amdgpu_ras_interrupt_dispatch(adev, &ih_data); > return 0; > diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c > b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c > index dafe91770863..b9c009236a33 100644 > --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c > @@ -297,10 +297,16 @@ static int gmc_v9_0_process_ecc_irq(struct > amdgpu_device *adev, > struct amdgpu_irq_src *source, > struct amdgpu_iv_entry *entry) > { > + struct ras_common_if *ras_if = adev->gmc.ras_if; > struct ras_dispatch_if ih_data = { > - .head = *adev->gmc.ras_if, > .entry = entry, > }; > + > + if (!ras_if) > + return 0; > + > + ih_data.head = *ras_if; > + > amdgpu_ras_interrupt_dispatch(adev, &ih_data); > return 0; > } > diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c > b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c > index 2a2025f548a1..304d7ba0772c 100644 > --- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c > @@ -1865,10 +1865,16 @@ static int sdma_v4_0_process_ecc_irq(struct > amdgpu_device *adev, > struct amdgpu_irq_src *source, > struct amdgpu_iv_entry *entry) { > + struct ras_common_if *ras_if = adev->sdma.ras_if; > struct ras_dispatch_if ih_data = { > - .head = *adev->sdma.ras_if, > .entry = entry, > }; > + > + if (!ras_if) > + return 0; > + > + ih_data.head = *ras_if; > + > amdgpu_ras_interrupt_dispatch(adev, &ih_data); > return 0; > } > -- > 2.17.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
RE: [PATCH 2/2] drm/ttm: fix start page for huge page check in ttm_put_pages()
> -Original Message- > From: Christian König [mailto:ckoenig.leichtzumer...@gmail.com] > Sent: Monday, April 08, 2019 9:13 PM > To: Zhang, Jerry ; Huang, Ray > ; amd-gfx@lists.freedesktop.org; dri- > de...@lists.freedesktop.org > Subject: [PATCH 2/2] drm/ttm: fix start page for huge page check in > ttm_put_pages() > > The first page entry is always the same with itself. > > Signed-off-by: Christian König Reviewed-by: Huang Rui > --- > drivers/gpu/drm/ttm/ttm_page_alloc.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc.c > b/drivers/gpu/drm/ttm/ttm_page_alloc.c > index f77c81db161b..c74147f0cbe3 100644 > --- a/drivers/gpu/drm/ttm/ttm_page_alloc.c > +++ b/drivers/gpu/drm/ttm/ttm_page_alloc.c > @@ -732,7 +732,7 @@ static void ttm_put_pages(struct page **pages, > unsigned npages, int flags, #ifdef CONFIG_TRANSPARENT_HUGEPAGE > if (!(flags & TTM_PAGE_FLAG_DMA32) && > (npages - i) >= HPAGE_PMD_NR) { > - for (j = 0; j < HPAGE_PMD_NR; ++j) > + for (j = 1; j < HPAGE_PMD_NR; ++j) > if (p++ != pages[i + j]) > break; > > @@ -767,7 +767,7 @@ static void ttm_put_pages(struct page **pages, > unsigned npages, int flags, > if (!p) > break; > > - for (j = 0; j < HPAGE_PMD_NR; ++j) > + for (j = 1; j < HPAGE_PMD_NR; ++j) > if (p++ != pages[i + j]) > break; > > -- > 2.17.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
RE: [PATCH 1/2] drm/ttm: fix out-of-bounds read in ttm_put_pages() v2
> -Original Message- > From: Christian König [mailto:ckoenig.leichtzumer...@gmail.com] > Sent: Monday, April 08, 2019 9:13 PM > To: Zhang, Jerry ; Huang, Ray > ; amd-gfx@lists.freedesktop.org; dri- > de...@lists.freedesktop.org > Subject: [PATCH 1/2] drm/ttm: fix out-of-bounds read in ttm_put_pages() v2 > > When ttm_put_pages() tries to figure out whether it's dealing with > transparent hugepages, it just reads past the bounds of the pages array > without a check. > > v2: simplify the test if enough pages are left in the array (Christian). > > Signed-off-by: Jann Horn > Signed-off-by: Christian König Reviewed-by: Huang Rui > Fixes: 5c42c64f7d54 ("drm/ttm: fix the fix for huge compound pages") > Cc: sta...@vger.kernel.org > --- > drivers/gpu/drm/ttm/ttm_page_alloc.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc.c > b/drivers/gpu/drm/ttm/ttm_page_alloc.c > index f841accc2c00..f77c81db161b 100644 > --- a/drivers/gpu/drm/ttm/ttm_page_alloc.c > +++ b/drivers/gpu/drm/ttm/ttm_page_alloc.c > @@ -730,7 +730,8 @@ static void ttm_put_pages(struct page **pages, > unsigned npages, int flags, > } > > #ifdef CONFIG_TRANSPARENT_HUGEPAGE > - if (!(flags & TTM_PAGE_FLAG_DMA32)) { > + if (!(flags & TTM_PAGE_FLAG_DMA32) && > + (npages - i) >= HPAGE_PMD_NR) { > for (j = 0; j < HPAGE_PMD_NR; ++j) > if (p++ != pages[i + j]) > break; > @@ -759,7 +760,7 @@ static void ttm_put_pages(struct page **pages, > unsigned npages, int flags, > unsigned max_size, n2free; > > spin_lock_irqsave(&huge->lock, irq_flags); > - while (i < npages) { > + while ((npages - i) >= HPAGE_PMD_NR) { > struct page *p = pages[i]; > unsigned j; > > -- > 2.17.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
RE: [PATCH 1/2] drm/amd/powerplay: optimization function of smu_update_table
> -Original Message- > From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On Behalf > Of Wang, Kevin(Yang) > Sent: Monday, April 08, 2019 4:43 PM > To: amd-gfx@lists.freedesktop.org > Cc: Huang, Ray ; Feng, Kenneth > ; Wang, Kevin(Yang) > Subject: [PATCH 1/2] drm/amd/powerplay: optimization function of > smu_update_table > > in fact, the firmware need 2 parameter: 1.table_id, 2.XferArg so change the > function interface to match the firmware code > > Signed-off-by: Kevin Wang Series are Reviewed-by: Huang Rui > --- > drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 7 +-- > drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h | 5 - > 2 files changed, 9 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c > b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c > index 7e8c74da6a74..c058c784180e 100644 > --- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c > +++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c > @@ -97,16 +97,19 @@ int smu_common_read_sensor(struct smu_context > *smu, enum amd_pp_sensors sensor, > return ret; > } > > -int smu_update_table(struct smu_context *smu, uint32_t table_id, > +int smu_update_table_with_arg(struct smu_context *smu, uint16_t > +table_id, uint16_t exarg, >void *table_data, bool drv2smu) > { > struct smu_table_context *smu_table = &smu->smu_table; > struct smu_table *table = NULL; > int ret = 0; > + uint32_t table_index; > > if (!table_data || table_id >= smu_table->table_count) > return -EINVAL; > > + table_index = (exarg << 16) | table_id; > + > table = &smu_table->tables[table_id]; > > if (drv2smu) > @@ -123,7 +126,7 @@ int smu_update_table(struct smu_context *smu, > uint32_t table_id, > ret = smu_send_smc_msg_with_param(smu, drv2smu ? > > SMU_MSG_TransferTableDram2Smu : > > SMU_MSG_TransferTableSmu2Dram, > - table_id); > + table_index); > if (ret) > return ret; > > diff --git a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h > b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h > index 2083139533e9..c146b5e884f8 100644 > --- a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h > +++ b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h > @@ -745,8 +745,11 @@ extern int smu_feature_set_enabled(struct > smu_context *smu, int feature_id, bool extern int > smu_feature_is_supported(struct smu_context *smu, int feature_id); > extern int smu_feature_set_supported(struct smu_context *smu, int > feature_id, bool enable); > > -int smu_update_table(struct smu_context *smu, uint32_t table_id, > +int smu_update_table_with_arg(struct smu_context *smu, uint16_t > +table_id, uint16_t exarg, >void *table_data, bool drv2smu); > +#define smu_update_table(smu, table_id, table_data, drv2smu) \ > + smu_update_table_with_arg((smu), (table_id), 0, (table_data), > +(drv2smu)) > + > bool is_support_sw_smu(struct amdgpu_device *adev); int > smu_reset(struct smu_context *smu); int > smu_common_read_sensor(struct smu_context *smu, enum > amd_pp_sensors sensor, > -- > 2.21.0 > > ___ > amd-gfx mailing list > amd-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
RE: Screen flicckering due to "drm/amdgpu: enable gfxoff again on raven series"
> -Original Message- > From: Pinky [mailto:j...@seznam.cz] > Sent: Monday, April 08, 2019 8:42 PM > To: Huang, Ray > Cc: Alex Deucher ; Deucher, Alexander > ; amd-gfx@lists.freedesktop.org > Subject: Re: Screen flicckering due to "drm/amdgpu: enable gfxoff again on > raven series" > > * Huang, Ray [2019-04-08 10:36:53 +]: > > >> -Original Message- > >> From: Pinky [mailto:j...@seznam.cz] > >> Sent: Saturday, April 06, 2019 3:09 PM > >> To: Alex Deucher > >> Cc: Deucher, Alexander ; Huang, Ray > >> ; amd-gfx@lists.freedesktop.org > >> Subject: Re: Screen flicckering due to "drm/amdgpu: enable gfxoff > >> again on raven series" > >> > >> Yes this seems to solve the issue, even > >> amdgpu.ppfeaturemask=0x7fff provides the same efect. > >> > >> > >> * Alex Deucher [2019-04-05 16:26:15 -0400]: > >> > >> >On Fri, Apr 5, 2019 at 4:00 PM Pinky wrote: > >> >> > >> >> Yes, this one seems to be already in the drm-next branch (HEAD is > >> >> now 7cdd4dc58d28). > >> >> > >> > > >> >I'm not sure what is going on them. You have RLC firmware version > >> >53815 (0xd237) so gfx power gating is being disabled as of > >> >083d022913f2f8c3bc1183a13874ad777b9f5bdd. > > > >Hi Pinkava, > > > >We have a similar issue that screen flickering caused by stutter mode. > > > >But per your testing result, I am a bit confused. > > Hello Ray, > > sorry for confussion, I re-tested it, there are two separate bugs: > > flickering - probably related to stutter mode bug >(white flickering stripes on screen, ) > > artifacts - the original issue i have reported (broken but steady image) > > 5.1.0-rc2-1-next-git-g7cdd4dc58d28 - artifacts, flickering > amdgpu.ppfeaturemask=0x7fff - flickering > amdgpu.ppfeaturemask=0xfffd - artifacts > amdgpu.ppfeaturemask=0xfffd7fff - OK > > amdgpu.ppfeaturemask=0xbfff - artifacts, flickering > amdgpu.ppfeaturemask=0xfffdbfff - artifacts > amdgpu.ppfeaturemask=0xfffd3fff - OK > > Sorry again for confussion. Is there a patch for the shutter so I can test it? > OK. Hersen, would you mind to share your patch to Pinkava? He want to give a test. Thanks, Ray > With best regards, > Pinkava J. > > > > >You mentioned that "ppfeaturemask=0xfffd3fff" could be fixed the issue, > but "0xfffdbfff" doesn't fix it. > >However, this is the same configuration while you add below patch. > Because the gfxoff feature will be disabled while detect the RLC ucode is > 53815 (0xd237). > >https://cgit.freedesktop.org/~agd5f/linux/commit/?h=drm-next-5.2- > wip&id > >=083d022913f2f8c3bc1183a13874ad777b9f5bdd > > > >Thanks, > >Ray > > > >> > > >> >Does setting amdgpu.ppfeaturemask=0xfffd3fff fix the issue? > >> > > >> >Alex > >> > > >> >> Just for completenes, there seems to be few more (probably > >> >> unrelated) > >> bugs. > >> >> One with mouse (1), some artifact sometimes appear and terminal in > >> >> X sometimes display black instead of text when scrooling. I did > >> >> not intend to discuss this here, just mentioning it. > >> >> > >> >> P > >> >> > >> >> > >> >> > >> >> (1) https://bugs.freedesktop.org/show_bug.cgi?id=110339 > >> >> > >> >> * Deucher, Alexander [2019-04-05 > >> 18:44:52 +]: > >> >> > >> >> >Make sure you have this patch as well: > >> >> >https://cgit.freedesktop.org/~agd5f/linux/commit/?h=drm-next-5.2- > >> wip > >> >> >&id=083d022913f2f8c3bc1183a13874ad777b9f5bdd > >> >> > > >> >> >Alex > >> >> > > >> >> >From: Pinky > >> >> >Sent: Friday, April 5, 2019 2:38 PM > >> >> >To: Deucher, Alexander > >> >> >Cc: Huang, Ray; amd-gfx@lists.freedesktop.org > >> >> >Subject: Re: Screen flicckering due to "drm/amdgpu: enable gfxoff > >> >> >again > >> on raven series" > >> >> > > >> >> > > >> >> >cat /sys/kernel/debug/dri/0/amdgpu_firmware_info > >> >> >VCE feature version: 0, firmware version: 0x UVD feature > >> >> >version: 0, firmware version: 0x MC feature version: 0, > >> >> >firmware version: 0x ME feature version: 40, firmware > >> >> >version: 0x0099 PFP feature version: 40, firmware version: > >> >> >0x00ae CE feature version: 40, firmware version: 0x004d > >> >> >RLC feature version: 1, firmware version: 0xd237 RLC SRLC > >> >> >feature > >> >> >version: 1, firmware version: 0x0001 RLC SRLG feature version: > >> >> >1, firmware version: 0x0001 RLC SRLS feature version: 1, > >> >> >firmware version: 0x0001 MEC feature version: 40, firmware > >> >> >version: 0x018b > >> >> >MEC2 feature version: 40, firmware version: 0x018b SOS > >> >> >feature > >> >> >version: 0, firmware version: 0x ASD feature version: 0, > >> >> >firmware version: 0x0017ba78 TA XGMI feature version: 0, firmware > >> >> >version: 0x TA RAS feature version: 0, firmware version: > >> >> >0x SMC feature version: 0, firmware version: 0x1e44 > >> >> >SDMA0 feature version: 41, firmware version: 0x00a9 VCN > >> >> >feature > >> >> >version
Re: [PATCH v2] drm/amd/display: Initialize stream_update with memset
On 2019-03-22 9:59 a.m., Nicholas Kazlauskas wrote: > The brace initialization used here generates warnings on some > compilers. For example, on GCC 4.9: > > [...] In function ‘dm_determine_update_type_for_commit’: > [...] error: missing braces around initializer [-Werror=missing-braces] > struct dc_stream_update stream_update = { 0 }; >^ > > Use memset to make this more portable. > > v2: Specify the compiler / diagnostic in the commit message (Paul) > > Cc: Sun peng Li > Cc: Harry Wentland > Signed-off-by: Nicholas Kazlauskas Reviewed-by: Leo Li > --- > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > 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 5b7a85e28fab..9cdd52edfc3d 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > @@ -5982,7 +5982,9 @@ dm_determine_update_type_for_commit(struct dc *dc, > } > > for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, > new_crtc_state, i) { > - struct dc_stream_update stream_update = { 0 }; > + struct dc_stream_update stream_update; > + > + memset(&stream_update, 0, sizeof(stream_update)); > > new_dm_crtc_state = to_dm_crtc_state(new_crtc_state); > old_dm_crtc_state = to_dm_crtc_state(old_crtc_state); > ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amd/display: Remove semicolon from to_dm_plane_state definition
On 2019-03-25 3:27 p.m., Nicholas Kazlauskas wrote: > The extra ; in the macro definition creates an empty statement > preventing any variable declarations from occuring after > any use of to_dm_plane_state(...). > > Signed-off-by: Nicholas Kazlauskas Reviewed-by: Leo Li > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h > b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h > index 87ca5746f861..94b77488a0e4 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h > @@ -57,7 +57,7 @@ struct amdgpu_hpd; > #define to_amdgpu_encoder(x) container_of(x, struct amdgpu_encoder, base) > #define to_amdgpu_framebuffer(x) container_of(x, struct amdgpu_framebuffer, > base) > > -#define to_dm_plane_state(x) container_of(x, struct dm_plane_state, base); > +#define to_dm_plane_state(x) container_of(x, struct dm_plane_state, base) > > #define AMDGPU_MAX_HPD_PINS 6 > #define AMDGPU_MAX_CRTCS 6 > ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 2/3] drm/sched: Keep s_fence->parent pointer
Am 08.04.19 um 18:08 schrieb Andrey Grodzovsky: For later driver's reference to see if the fence is signaled. Signed-off-by: Andrey Grodzovsky --- drivers/gpu/drm/scheduler/sched_main.c | 11 --- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c index c215cde..5bb4368 100644 --- a/drivers/gpu/drm/scheduler/sched_main.c +++ b/drivers/gpu/drm/scheduler/sched_main.c @@ -191,8 +191,6 @@ EXPORT_SYMBOL(drm_sched_dependency_optimized); */ static void drm_sched_start_timeout(struct drm_gpu_scheduler *sched) { - unsigned long flags; - I think this actually belongs into patch #1. if (sched->timeout != MAX_SCHEDULE_TIMEOUT && !list_empty(&sched->ring_mirror_list)) schedule_delayed_work(&sched->work_tdr, sched->timeout); @@ -371,7 +369,6 @@ void drm_sched_stop(struct drm_gpu_scheduler *sched, struct drm_sched_job *bad) dma_fence_remove_callback(s_job->s_fence->parent, &s_job->cb)) { dma_fence_put(s_job->s_fence->parent); - s_job->s_fence->parent = NULL; How about also moving the dma_fence_put() into drm_sched_resubmit_jobs(), right before we re-assign s_job->s_fence->parent? I think that would be cleaner, but not sure if that wouldn't have any ugly side effects. Christian. atomic_dec(&sched->hw_rq_count); } else { /* @@ -398,6 +395,14 @@ void drm_sched_stop(struct drm_gpu_scheduler *sched, struct drm_sched_job *bad) sched->ops->free_job(s_job); } } + + /* +* Stop pending timer in flight as we rearm it in drm_sched_start. This +* avoids the pending timeout work in progress to fire right away after +* this TDR finished and before the newly restarted jobs had a +* chance to complete. +*/ + cancel_delayed_work(&sched->work_tdr); } EXPORT_SYMBOL(drm_sched_stop); ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 3/3] drm/amdgpu: Avoid HW reset if guilty job already signaled.
Am 08.04.19 um 18:08 schrieb Andrey Grodzovsky: Also reject TDRs if another one already running. Signed-off-by: Andrey Grodzovsky --- drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 94 +- 1 file changed, 67 insertions(+), 27 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c index aabd043..4446892 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c @@ -3327,10 +3327,12 @@ bool amdgpu_device_should_recover_gpu(struct amdgpu_device *adev) static int amdgpu_device_pre_asic_reset(struct amdgpu_device *adev, struct amdgpu_job *job, - bool *need_full_reset_arg) + bool *need_full_reset_arg, + bool *job_signaled) { int i, r = 0; bool need_full_reset = *need_full_reset_arg; + struct amdgpu_ring *job_ring = job ? to_amdgpu_ring(job->base.sched) : NULL; /* block all schedulers and reset given job's ring */ for (i = 0; i < AMDGPU_MAX_RINGS; ++i) { @@ -3341,6 +3343,17 @@ static int amdgpu_device_pre_asic_reset(struct amdgpu_device *adev, drm_sched_stop(&ring->sched, &job->base); + /* +* Must check guilty signal here since after this point all old +* HW fences are force signaled. +* +* job->base holds a reference to parent fence +*/ + if (job_signaled && job && ring == job_ring && + job->base.s_fence->parent && + dma_fence_is_signaled(job->base.s_fence->parent)) + *job_signaled = true; + That won't work correctly. See when the guilty job is not on the first scheduler, you would already have force completed some before getting here. Better to stop all schedulers first and then do the check. Christian. /* after all hw jobs are reset, hw fence is meaningless, so force_completion */ amdgpu_fence_driver_force_completion(ring); } @@ -3358,7 +3371,8 @@ static int amdgpu_device_pre_asic_reset(struct amdgpu_device *adev, - if (!amdgpu_sriov_vf(adev)) { + /* Don't suspend on bare metal if we are not going to HW reset the ASIC */ + if (!amdgpu_sriov_vf(adev) && !(*job_signaled)) { if (!need_full_reset) need_full_reset = amdgpu_device_ip_need_full_reset(adev); @@ -3495,7 +3509,7 @@ static int amdgpu_do_asic_reset(struct amdgpu_hive_info *hive, return r; } -static void amdgpu_device_post_asic_reset(struct amdgpu_device *adev) +static void amdgpu_device_post_asic_reset(struct amdgpu_device *adev, bool job_signaled) { int i; @@ -3505,7 +3519,8 @@ static void amdgpu_device_post_asic_reset(struct amdgpu_device *adev) if (!ring || !ring->sched.thread) continue; - if (!adev->asic_reset_res) + /* No point to resubmit jobs if we didn't HW reset*/ + if (!adev->asic_reset_res && !job_signaled) drm_sched_resubmit_jobs(&ring->sched); drm_sched_start(&ring->sched, !adev->asic_reset_res); @@ -3518,14 +3533,21 @@ static void amdgpu_device_post_asic_reset(struct amdgpu_device *adev) adev->asic_reset_res = 0; } -static void amdgpu_device_lock_adev(struct amdgpu_device *adev) +static bool amdgpu_device_lock_adev(struct amdgpu_device *adev, bool trylock) { - mutex_lock(&adev->lock_reset); + if (trylock) { + if (!mutex_trylock(&adev->lock_reset)) + return false; + } else + mutex_lock(&adev->lock_reset); + atomic_inc(&adev->gpu_reset_counter); adev->in_gpu_reset = 1; /* Block kfd: SRIOV would do it separately */ if (!amdgpu_sriov_vf(adev)) amdgpu_amdkfd_pre_reset(adev); + + return true; } static void amdgpu_device_unlock_adev(struct amdgpu_device *adev) @@ -3555,29 +3577,44 @@ int amdgpu_device_gpu_recover(struct amdgpu_device *adev, { int r; struct amdgpu_hive_info *hive = NULL; - bool need_full_reset = false; struct amdgpu_device *tmp_adev = NULL; struct list_head device_list, *device_list_handle = NULL; + bool xgmi_topology_present, need_full_reset, job_signaled; + need_full_reset = job_signaled = false; INIT_LIST_HEAD(&device_list); dev_info(adev->dev, "GPU reset begin!\n"); + hive = amdgpu_get_xgmi_hive(adev, 0); + xgmi_topology_present = hive && adev->gmc.xgmi.num_physical_nodes > 1; + /* -* In case of XGMI hive disallow concurrent resets to be triggered -* by different nodes. No point also since the one node already executing -* reset will also rese
Re: [PATCH] drm/amdgpu: Always enable memory sharing within same XGMI hive
ping On 2019-04-08 6:32 p.m., Liu, Shaoyun wrote: > XGMI Memory sharing will be disbaled by default for security reason after > boot up, it depends on driver to enable the memory sharing > > Change-Id: Ib516066eecfb877f84f1460a4d659abea44adb02 > Signed-off-by: shaoyunl > --- > drivers/gpu/drm/amd/amdgpu/psp_v11_0.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c > b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c > index 2b3429d..b91df7b 100644 > --- a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c > @@ -674,7 +674,7 @@ static int psp_v11_0_xgmi_set_topology_info(struct > psp_context *psp, > for (i = 0; i < topology_info_input->num_nodes; i++) { > topology_info_input->nodes[i].node_id = > topology->nodes[i].node_id; > topology_info_input->nodes[i].num_hops = > topology->nodes[i].num_hops; > - topology_info_input->nodes[i].is_sharing_enabled = > topology->nodes[i].is_sharing_enabled; > + topology_info_input->nodes[i].is_sharing_enabled = 1; > topology_info_input->nodes[i].sdma_engine = > topology->nodes[i].sdma_engine; > } > ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 1/2] drm/powerplay: Add smu set xgmi pstate interface
ping On 2019-04-05 12:01 p.m., Liu, Shaoyun wrote: > XGMI pstate is controlled by SMU, driver need this interface to communicate > with SMU > > Change-Id: I3a30797332557725d48d392bea0c9d59e2d0e427 > Signed-off-by: shaoyunl > --- > drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h | 4 > drivers/gpu/drm/amd/powerplay/smu_v11_0.c | 8 > 2 files changed, 12 insertions(+) > > diff --git a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h > b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h > index 2083139..0e52d1b 100644 > --- a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h > +++ b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h > @@ -546,6 +546,8 @@ struct smu_funcs > int (*get_fan_speed_percent)(struct smu_context *smu, uint32_t *speed); > int (*set_fan_speed_percent)(struct smu_context *smu, uint32_t speed); > int (*set_fan_speed_rpm)(struct smu_context *smu, uint32_t speed); > + int (*set_xgmi_pstate)(struct smu_context *smu, uint32_t pstate); > + > }; > > #define smu_init_microcode(smu) \ > @@ -729,6 +731,8 @@ struct smu_funcs > ((smu)->funcs->get_sclk ? (smu)->funcs->get_sclk((smu), (low)) : 0) > #define smu_get_mclk(smu, low) \ > ((smu)->funcs->get_mclk ? (smu)->funcs->get_mclk((smu), (low)) : 0) > +#define smu_set_xgmi_pstate(smu, pstate) \ > + ((smu)->funcs->set_xgmi_pstate ? > (smu)->funcs->set_xgmi_pstate((smu), (pstate)) : 0) > > > extern int smu_get_atom_data_table(struct smu_context *smu, uint32_t table, > diff --git a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c > b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c > index 9d75911..2e2ce71 100644 > --- a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c > +++ b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c > @@ -1952,6 +1952,13 @@ static int smu_v11_0_set_fan_speed_rpm(struct > smu_context *smu, > return ret; > } > > +static int smu_v11_0_set_xgmi_pstate(struct smu_context *smu, > + uint32_t pstate) > +{ > + /* send msg to SMU to set pstate */ > + return 0; > +} > + > static const struct smu_funcs smu_v11_0_funcs = { > .init_microcode = smu_v11_0_init_microcode, > .load_microcode = smu_v11_0_load_microcode, > @@ -2008,6 +2015,7 @@ static const struct smu_funcs smu_v11_0_funcs = { > .get_fan_speed_percent = smu_v11_0_get_fan_speed_percent, > .set_fan_speed_percent = smu_v11_0_set_fan_speed_percent, > .set_fan_speed_rpm = smu_v11_0_set_fan_speed_rpm, > + .set_xgmi_pstate = smu_v11_0_set_xgmi_pstate, > }; > > void smu_v11_0_set_smu_funcs(struct smu_context *smu) ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 2/2] drm/amdgpu: Set proper function to set xgmi pstate
ping On 2019-04-05 12:01 p.m., Liu, Shaoyun wrote: > Driver need to call SMU to set xgmi pstate > > Change-Id: Iad7fd0e3b3155e45be8fe9119686c5bafa3c176c > Signed-off-by: shaoyunl > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c | 13 - > 1 file changed, 12 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c > index 3368347..a48c84c 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c > @@ -24,6 +24,7 @@ > #include > #include "amdgpu.h" > #include "amdgpu_xgmi.h" > +#include "amdgpu_smu.h" > > > static DEFINE_MUTEX(xgmi_mutex); > @@ -216,7 +217,17 @@ int amdgpu_xgmi_set_pstate(struct amdgpu_device *adev, > int pstate) > > if (hive->pstate == pstate) > return 0; > - /* Todo : sent the message to SMU for pstate change */ > + > + dev_dbg(adev->dev, "Set xgmi pstate %d.\n", pstate); > + > + if (is_support_sw_smu(adev)) > + ret = smu_set_xgmi_pstate(&adev->smu, pstate); > + if (ret) > + dev_err(adev->dev, > + "XGMI: Set pstate failure on device %llx, hive %llx, > ret %d", > + adev->gmc.xgmi.node_id, > + adev->gmc.xgmi.hive_id, ret); > + > return ret; > } > ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amdgpu: support dpm level modification under virtualization
Under vega10 virtualuzation, smu ip block will not be added. Therefore, we need add pp clk query and force dpm level function at amdgpu_virt_ops to support the feature. Change-Id: I713419c57b854082f6f739f1d32a055c7115e620 Signed-off-by: Yintian Tao --- drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 1 + drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c| 3 ++ drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 15 ++ drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c | 33 + drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h | 11 + drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c | 78 ++ drivers/gpu/drm/amd/amdgpu/mxgpu_ai.h | 6 +++ 7 files changed, 147 insertions(+) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c index 3ff8899..bb0fd5a 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c @@ -2486,6 +2486,7 @@ int amdgpu_device_init(struct amdgpu_device *adev, mutex_init(&adev->virt.vf_errors.lock); hash_init(adev->mn_hash); mutex_init(&adev->lock_reset); + mutex_init(&adev->virt.dpm_mutex); amdgpu_device_check_arguments(adev); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c index 6190495..1353955 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c @@ -727,6 +727,9 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file if (adev->pm.dpm_enabled) { dev_info.max_engine_clock = amdgpu_dpm_get_sclk(adev, false) * 10; dev_info.max_memory_clock = amdgpu_dpm_get_mclk(adev, false) * 10; + } else if (amdgpu_sriov_vf(adev)) { + dev_info.max_engine_clock = amdgpu_virt_get_sclk(adev, false) * 10; + dev_info.max_memory_clock = amdgpu_virt_get_mclk(adev, false) * 10; } else { dev_info.max_engine_clock = adev->clock.default_sclk * 10; dev_info.max_memory_clock = adev->clock.default_mclk * 10; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c index 5540259..0162d1e 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c @@ -380,6 +380,17 @@ static ssize_t amdgpu_set_dpm_forced_performance_level(struct device *dev, goto fail; } +if (amdgpu_sriov_vf(adev)) { +if (amdgim_is_hwperf(adev) && +adev->virt.ops->force_dpm_level) { +mutex_lock(&adev->pm.mutex); +adev->virt.ops->force_dpm_level(adev, level); +mutex_unlock(&adev->pm.mutex); +return count; +} else +return -EINVAL; +} + if (current_level == level) return count; @@ -843,6 +854,10 @@ static ssize_t amdgpu_get_pp_dpm_sclk(struct device *dev, struct drm_device *ddev = dev_get_drvdata(dev); struct amdgpu_device *adev = ddev->dev_private; + if (amdgpu_sriov_vf(adev) && amdgim_is_hwperf(adev) && + adev->virt.ops->get_pp_clk) + return adev->virt.ops->get_pp_clk(adev, PP_SCLK, buf); + if (is_support_sw_smu(adev)) return smu_print_clk_levels(&adev->smu, PP_SCLK, buf); else if (adev->powerplay.pp_funcs->print_clock_levels) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c index 462a04e..ae4b2a1 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c @@ -375,4 +375,37 @@ void amdgpu_virt_init_data_exchange(struct amdgpu_device *adev) } } +static uint32_t parse_clk(char *buf, bool min) +{ +char *ptr = buf; +uint32_t clk = 0; + +do { +ptr = strchr(ptr, ':'); +if (!ptr) +break; +ptr+=2; +clk = simple_strtoul(ptr, NULL, 10); +} while (!min); + +return clk * 100; +} + +uint32_t amdgpu_virt_get_sclk(struct amdgpu_device *adev, bool lowest) +{ +char buf[512] = {0}; + +adev->virt.ops->get_pp_clk(adev, PP_SCLK, buf); + +return parse_clk(buf, lowest); +} + +uint32_t amdgpu_virt_get_mclk(struct amdgpu_device *adev, bool lowest) +{ +char buf[512] = {0}; + +adev->virt.ops->get_pp_clk(adev, PP_MCLK, buf); + +return parse_clk(buf, lowest); +} diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h index 722deef..584947b 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h @@ -57,6 +57,8 @@ struct amdgpu_virt_ops { int (*reset_gpu)(struct amdgp
Re: [PATCH v2] drm/amd/display: Add debugfs entry for amdgpu_dm_visual_confirm
On 2019-03-22 10:11 a.m., Nicholas Kazlauskas wrote: > [Why] > DC provides a few visual confirmation debug options that can be > dynamically changed at runtime to help debug surface programming issues > but we don't have any way to access it from userspace. > > [How] > Add the amdgpu_dm_visual_confirm debugfs entry. > It accepts a string containing the DC visual confirm enum value using > the debugfs attribute helpers. > > The debugfs_create_file_unsafe can be used instead of > debugfs_create_file as per the documentation. > > v2: Use debugfs helpers for getting and setting the value (Christian) > > Cc: Leo Li Reviewed-by: Leo Li Thanks, Leo > Cc: Harry Wentland > Signed-off-by: Nicholas Kazlauskas > --- > .../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 39 ++- > 1 file changed, 38 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c > index 1a9e3d3dfa38..1d5fc5ad3bee 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c > @@ -995,6 +995,35 @@ static const struct drm_info_list > amdgpu_dm_debugfs_list[] = { > {"amdgpu_target_backlight_pwm", &target_backlight_read}, > }; > > +/* > + * Sets the DC visual confirm debug option from the given string. > + * Example usage: echo 1 > /sys/kernel/debug/dri/0/amdgpu_visual_confirm > + */ > +static int visual_confirm_set(void *data, u64 val) > +{ > + struct amdgpu_device *adev = data; > + > + adev->dm.dc->debug.visual_confirm = (enum visual_confirm)val; > + > + return 0; > +} > + > +/* > + * Reads the DC visual confirm debug option value into the given buffer. > + * Example usage: cat /sys/kernel/debug/dri/0/amdgpu_dm_visual_confirm > + */ > +static int visual_confirm_get(void *data, u64 *val) > +{ > + struct amdgpu_device *adev = data; > + > + *val = adev->dm.dc->debug.visual_confirm; > + > + return 0; > +} > + > +DEFINE_DEBUGFS_ATTRIBUTE(visual_confirm_fops, visual_confirm_get, > + visual_confirm_set, "%llu\n"); > + > int dtn_debugfs_init(struct amdgpu_device *adev) > { > static const struct file_operations dtn_log_fops = { > @@ -1020,5 +1049,13 @@ int dtn_debugfs_init(struct amdgpu_device *adev) > adev, > &dtn_log_fops); > > - return PTR_ERR_OR_ZERO(ent); > + if (IS_ERR(ent)) > + return PTR_ERR(ent); > + > + ent = debugfs_create_file_unsafe("amdgpu_dm_visual_confirm", 0644, root, > + adev, &visual_confirm_fops); > + if (IS_ERR(ent)) > + return PTR_ERR(ent); > + > + return 0; > } > ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH RFC tip/core/rcu 0/4] Forbid static SRCU use in modules
On Mon, Apr 08, 2019 at 01:24:47PM -0400, Mathieu Desnoyers wrote: > - On Apr 8, 2019, at 11:46 AM, paulmck paul...@linux.ibm.com wrote: > > > On Mon, Apr 08, 2019 at 10:49:32AM -0400, Mathieu Desnoyers wrote: > >> - On Apr 8, 2019, at 10:22 AM, paulmck paul...@linux.ibm.com wrote: > >> > >> > On Mon, Apr 08, 2019 at 09:05:34AM -0400, Mathieu Desnoyers wrote: > >> >> - On Apr 7, 2019, at 10:27 PM, paulmck paul...@linux.ibm.com wrote: > >> >> > >> >> > On Sun, Apr 07, 2019 at 09:07:18PM +, Joel Fernandes wrote: > >> >> >> On Sun, Apr 07, 2019 at 04:41:36PM -0400, Mathieu Desnoyers wrote: > >> >> >> > > >> >> >> > - On Apr 7, 2019, at 3:32 PM, Joel Fernandes, Google > >> >> >> > j...@joelfernandes.org > >> >> >> > wrote: > >> >> >> > > >> >> >> > > On Sun, Apr 07, 2019 at 03:26:16PM -0400, Mathieu Desnoyers > >> >> >> > > wrote: > >> >> >> > >> - On Apr 7, 2019, at 9:59 AM, paulmck paul...@linux.ibm.com > >> >> >> > >> wrote: > >> >> >> > >> > >> >> >> > >> > On Sun, Apr 07, 2019 at 06:39:41AM -0700, Paul E. McKenney > >> >> >> > >> > wrote: > >> >> >> > >> >> On Sat, Apr 06, 2019 at 07:06:13PM -0400, Joel Fernandes > >> >> >> > >> >> wrote: > >> >> >> > >> > > >> >> >> > >> > [ . . . ] > >> >> >> > >> > > >> >> >> > >> >> > > diff --git a/include/asm-generic/vmlinux.lds.h > >> >> >> > >> >> > > b/include/asm-generic/vmlinux.lds.h > >> >> >> > >> >> > > index f8f6f04c4453..c2d919a1566e 100644 > >> >> >> > >> >> > > --- a/include/asm-generic/vmlinux.lds.h > >> >> >> > >> >> > > +++ b/include/asm-generic/vmlinux.lds.h > >> >> >> > >> >> > > @@ -338,6 +338,10 @@ > >> >> >> > >> >> > > KEEP(*(__tracepoints_ptrs)) /* > >> >> >> > >> >> > > Tracepoints: pointer array */ \ > >> >> >> > >> >> > > __stop___tracepoints_ptrs = .; > >> >> >> > >> >> > > \ > >> >> >> > >> >> > > *(__tracepoints_strings)/* Tracepoints: > >> >> >> > >> >> > > strings */ \ > >> >> >> > >> >> > > +. = ALIGN(8); > >> >> >> > >> >> > > \ > >> >> >> > >> >> > > +__start___srcu_struct = .; > >> >> >> > >> >> > > \ > >> >> >> > >> >> > > +*(___srcu_struct_ptrs) > >> >> >> > >> >> > > \ > >> >> >> > >> >> > > +__end___srcu_struct = .; > >> >> >> > >> >> > > \ > >> >> >> > >> >> > > } > >> >> >> > >> >> > > \ > >> >> >> > >> >> > > >> >> >> > >> >> > This vmlinux linker modification is not needed. I tested > >> >> >> > >> >> > without it and srcu > >> >> >> > >> >> > torture works fine with rcutorture built as a module. > >> >> >> > >> >> > Putting further prints > >> >> >> > >> >> > in kernel/module.c verified that the kernel is able to > >> >> >> > >> >> > find the srcu structs > >> >> >> > >> >> > just fine. You could squash the below patch into this one > >> >> >> > >> >> > or apply it on top > >> >> >> > >> >> > of the dev branch. > >> >> >> > >> >> > >> >> >> > >> >> Good point, given that otherwise FORTRAN named common blocks > >> >> >> > >> >> would not > >> >> >> > >> >> work. > >> >> >> > >> >> > >> >> >> > >> >> But isn't one advantage of leaving that stuff in the > >> >> >> > >> >> RO_DATA_SECTION() > >> >> >> > >> >> macro that it can be mapped read-only? Or am I suffering > >> >> >> > >> >> from excessive > >> >> >> > >> >> optimism? > >> >> >> > >> > > >> >> >> > >> > And to answer the other question, in the case where I am > >> >> >> > >> > suffering from > >> >> >> > >> > excessive optimism, it should be a separate commit. Please > >> >> >> > >> > see below > >> >> >> > >> > for the updated original commit thus far. > >> >> >> > >> > > >> >> >> > >> > And may I have your Tested-by? > >> >> >> > >> > >> >> >> > >> Just to confirm: does the cleanup performed in the modules going > >> >> >> > >> notifier end up acting as a barrier first before freeing the > >> >> >> > >> memory ? > >> >> >> > >> If not, is it explicitly stated that a barrier must be issued > >> >> >> > >> before > >> >> >> > >> module unload ? > >> >> >> > >> > >> >> >> > > > >> >> >> > > You mean rcu_barrier? It is mentioned in the documentation that > >> >> >> > > this is the > >> >> >> > > responsibility of the module writer to prevent delays for all > >> >> >> > > modules. > >> >> >> > > >> >> >> > It's a srcu barrier yes. Considering it would be a barrier > >> >> >> > specific to the > >> >> >> > srcu domain within that module, I don't see how it would cause > >> >> >> > delays for > >> >> >> > "all" modules if we implicitly issue the barrier on module unload. > >> >> >> > What > >> >> >> > am I missing ? > >> >> >> > >> >> >> Yes you are right. I thought of this after I just sent my email. I > >> >> >> think it > >> >> >> makes sense for srcu ca
Re: [PATCH 3/3] drm/amdgpu: Avoid HW reset if guilty job already signaled.
On 4/9/19 10:50 AM, Christian König wrote: > Am 08.04.19 um 18:08 schrieb Andrey Grodzovsky: >> Also reject TDRs if another one already running. >> >> Signed-off-by: Andrey Grodzovsky >> --- >> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 94 >> +- >> 1 file changed, 67 insertions(+), 27 deletions(-) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c >> index aabd043..4446892 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c >> @@ -3327,10 +3327,12 @@ bool amdgpu_device_should_recover_gpu(struct >> amdgpu_device *adev) >> static int amdgpu_device_pre_asic_reset(struct amdgpu_device *adev, >> struct amdgpu_job *job, >> - bool *need_full_reset_arg) >> + bool *need_full_reset_arg, >> + bool *job_signaled) >> { >> int i, r = 0; >> bool need_full_reset = *need_full_reset_arg; >> + struct amdgpu_ring *job_ring = job ? >> to_amdgpu_ring(job->base.sched) : NULL; >> /* block all schedulers and reset given job's ring */ >> for (i = 0; i < AMDGPU_MAX_RINGS; ++i) { >> @@ -3341,6 +3343,17 @@ static int amdgpu_device_pre_asic_reset(struct >> amdgpu_device *adev, >> drm_sched_stop(&ring->sched, &job->base); >> + /* >> + * Must check guilty signal here since after this point all old >> + * HW fences are force signaled. >> + * >> + * job->base holds a reference to parent fence >> + */ >> + if (job_signaled && job && ring == job_ring && >> + job->base.s_fence->parent && >> + dma_fence_is_signaled(job->base.s_fence->parent)) >> + *job_signaled = true; >> + > > That won't work correctly. See when the guilty job is not on the first > scheduler, you would already have force completed some before getting > here. > > Better to stop all schedulers first and then do the check. > > Christian. What do you mean by first scheduler ? There is one scheduler object per ring so I am not clear what 'first' means here. Andrey > >> /* after all hw jobs are reset, hw fence is meaningless, so >> force_completion */ >> amdgpu_fence_driver_force_completion(ring); >> } >> @@ -3358,7 +3371,8 @@ static int amdgpu_device_pre_asic_reset(struct >> amdgpu_device *adev, >> - if (!amdgpu_sriov_vf(adev)) { >> + /* Don't suspend on bare metal if we are not going to HW reset >> the ASIC */ >> + if (!amdgpu_sriov_vf(adev) && !(*job_signaled)) { >> if (!need_full_reset) >> need_full_reset = amdgpu_device_ip_need_full_reset(adev); >> @@ -3495,7 +3509,7 @@ static int amdgpu_do_asic_reset(struct >> amdgpu_hive_info *hive, >> return r; >> } >> -static void amdgpu_device_post_asic_reset(struct amdgpu_device *adev) >> +static void amdgpu_device_post_asic_reset(struct amdgpu_device >> *adev, bool job_signaled) >> { >> int i; >> @@ -3505,7 +3519,8 @@ static void >> amdgpu_device_post_asic_reset(struct amdgpu_device *adev) >> if (!ring || !ring->sched.thread) >> continue; >> - if (!adev->asic_reset_res) >> + /* No point to resubmit jobs if we didn't HW reset*/ >> + if (!adev->asic_reset_res && !job_signaled) >> drm_sched_resubmit_jobs(&ring->sched); >> drm_sched_start(&ring->sched, !adev->asic_reset_res); >> @@ -3518,14 +3533,21 @@ static void >> amdgpu_device_post_asic_reset(struct amdgpu_device *adev) >> adev->asic_reset_res = 0; >> } >> -static void amdgpu_device_lock_adev(struct amdgpu_device *adev) >> +static bool amdgpu_device_lock_adev(struct amdgpu_device *adev, bool >> trylock) >> { >> - mutex_lock(&adev->lock_reset); >> + if (trylock) { >> + if (!mutex_trylock(&adev->lock_reset)) >> + return false; >> + } else >> + mutex_lock(&adev->lock_reset); >> + >> atomic_inc(&adev->gpu_reset_counter); >> adev->in_gpu_reset = 1; >> /* Block kfd: SRIOV would do it separately */ >> if (!amdgpu_sriov_vf(adev)) >> amdgpu_amdkfd_pre_reset(adev); >> + >> + return true; >> } >> static void amdgpu_device_unlock_adev(struct amdgpu_device *adev) >> @@ -3555,29 +3577,44 @@ int amdgpu_device_gpu_recover(struct >> amdgpu_device *adev, >> { >> int r; >> struct amdgpu_hive_info *hive = NULL; >> - bool need_full_reset = false; >> struct amdgpu_device *tmp_adev = NULL; >> struct list_head device_list, *device_list_handle = NULL; >> + bool xgmi_topology_present, need_full_reset, job_signaled; >> + need_full_reset = job_signaled = false; >> INIT_LIST_HEAD(&device_list); >> dev_info(adev->dev, "GPU reset begin!\n"); >> + hive = amdgpu_get_xgmi_hive(adev, 0); >> + xgmi_topology_present = hive && >> adev->gm
[PATCH 00/20] DC Patches 09 Apr 2019
Summary Of changes *add color properties *fixes for determining update type *eDP link setting on detect Anthony Koo (1): drm/amd/display: Read eDP link settings on detection Aric Cyr (1): drm/amd/display: 3.2.26 David Francis (2): drm/amd/display: Handle get crtc position error drm/amd/display: If one stream full updates, full update all planes Joshua Aberback (2): drm/amd/display: Add fast_validate parameter drm/amd/display: Add profiling tools for bandwidth validation Jun Lei (1): drm/amd/display: expand plane caps to include fp16 and scaling capability Nicholas Kazlauskas (11): drm/amd/display: Expose support for NV12 on suitable planes drm/amd/display: Add DRM color properties for primary planes drm/amd/display: Update plane scaling parameters for fast updates drm/amd/display: Maintain z-ordering when creating planes drm/amd/display: Recalculate pitch when buffers change drm/amd/display: Rework DC plane filling and surface updates drm/amd/display: Add basic downscale and upscale valdiation drm/amd/display: Use surface directly when checking update type drm/amd/display: Don't warn when DC update type > DM guess drm/amd/display: Check scaling info when determing update type drm/amd/display: Relax requirements for CRTCs to be enabled Samson Tam (1): drm/amd/display: change name from dc_link_get_verified_link_cap to dc_link_get_link_cap Yongqiang Sun (1): drm/amd/display: define HUBP_MASK_SH_LIST_DCN for Raven .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 745 +++--- .../gpu/drm/amd/display/dc/calcs/dcn_calcs.c | 24 +- drivers/gpu/drm/amd/display/dc/core/dc.c | 21 +- .../gpu/drm/amd/display/dc/core/dc_link_dp.c | 33 +- .../gpu/drm/amd/display/dc/core/dc_resource.c | 6 +- .../gpu/drm/amd/display/dc/core/dc_stream.c | 3 +- drivers/gpu/drm/amd/display/dc/dc.h | 87 +- drivers/gpu/drm/amd/display/dc/dc_link.h | 2 +- .../amd/display/dc/dce100/dce100_resource.c | 22 +- .../amd/display/dc/dce110/dce110_resource.c | 41 +- .../amd/display/dc/dce112/dce112_resource.c | 22 +- .../amd/display/dc/dce112/dce112_resource.h | 3 +- .../amd/display/dc/dce120/dce120_resource.c | 19 +- .../drm/amd/display/dc/dce80/dce80_resource.c | 22 +- .../gpu/drm/amd/display/dc/dcn10/dcn10_hubp.h | 9 +- .../drm/amd/display/dc/dcn10/dcn10_resource.c | 20 +- .../gpu/drm/amd/display/dc/inc/core_types.h | 3 +- .../gpu/drm/amd/display/dc/inc/dcn_calcs.h| 3 +- 18 files changed, 760 insertions(+), 325 deletions(-) -- 2.17.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 04/20] drm/amd/display: Handle get crtc position error
From: David Francis [Why] dc_stream_get_crtc_position can return false. This was unhandled in delay_cursor_until_vupdate [How] If dc_stream_get_crtc_position returns false, something is weird. Don't delay. Change-Id: Id0fc61792aaa248594deb46d9984bcb3fb78559c Signed-off-by: David Francis Reviewed-by: Sun peng Li Acked-by: Bhawanpreet Lakha --- drivers/gpu/drm/amd/display/dc/core/dc_stream.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_stream.c b/drivers/gpu/drm/amd/display/dc/core/dc_stream.c index f7a293902234..e10479d58c11 100644 --- a/drivers/gpu/drm/amd/display/dc/core/dc_stream.c +++ b/drivers/gpu/drm/amd/display/dc/core/dc_stream.c @@ -211,7 +211,8 @@ static void delay_cursor_until_vupdate(struct pipe_ctx *pipe_ctx, struct dc *dc) ASIC_REV_IS_RAVEN(stream->ctx->asic_id.hw_internal_rev)) { vupdate_line = get_vupdate_offset_from_vsync(pipe_ctx); - dc_stream_get_crtc_position(dc, &stream, 1, &vpos, &nvpos); + if (!dc_stream_get_crtc_position(dc, &stream, 1, &vpos, &nvpos)) + return; if (vpos >= vupdate_line) return; -- 2.17.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 07/20] drm/amd/display: 3.2.26
From: Aric Cyr Change-Id: I2e35170195717fa417ddaeb372efe2908722d4a9 Signed-off-by: Aric Cyr Reviewed-by: Aric Cyr Acked-by: Bhawanpreet Lakha --- drivers/gpu/drm/amd/display/dc/dc.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/dc/dc.h b/drivers/gpu/drm/amd/display/dc/dc.h index 637a7b39eea4..b0fdf8387e7a 100644 --- a/drivers/gpu/drm/amd/display/dc/dc.h +++ b/drivers/gpu/drm/amd/display/dc/dc.h @@ -42,7 +42,7 @@ #include "inc/hw/dmcu.h" #include "dml/display_mode_lib.h" -#define DC_VER "3.2.25" +#define DC_VER "3.2.26" #define MAX_SURFACES 3 #define MAX_PLANES 6 -- 2.17.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 05/20] drm/amd/display: define HUBP_MASK_SH_LIST_DCN for Raven
From: Yongqiang Sun Change-Id: Ibe60745eaaea72d694ceb9d22936eae0abae5674 Signed-off-by: Yongqiang Sun Reviewed-by: Tony Cheng Acked-by: Bhawanpreet Lakha --- drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hubp.h | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hubp.h b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hubp.h index 3268ab089363..db98ba361686 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hubp.h +++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hubp.h @@ -247,7 +247,7 @@ .field_name = reg_name ## __ ## field_name ## post_fix /* Mask/shift struct generation macro for all ASICs (including those with reduced functionality) */ -#define HUBP_MASK_SH_LIST_DCN(mask_sh)\ +#define HUBP_MASK_SH_LIST_DCN_COMMON(mask_sh)\ HUBP_SF(HUBP0_DCHUBP_CNTL, HUBP_BLANK_EN, mask_sh),\ HUBP_SF(HUBP0_DCHUBP_CNTL, HUBP_TTU_DISABLE, mask_sh),\ HUBP_SF(HUBP0_DCHUBP_CNTL, HUBP_UNDERFLOW_STATUS, mask_sh),\ @@ -331,7 +331,6 @@ HUBP_SF(HUBP0_DCHUBP_REQ_SIZE_CONFIG, META_CHUNK_SIZE, mask_sh),\ HUBP_SF(HUBP0_DCHUBP_REQ_SIZE_CONFIG, MIN_META_CHUNK_SIZE, mask_sh),\ HUBP_SF(HUBP0_DCHUBP_REQ_SIZE_CONFIG, DPTE_GROUP_SIZE, mask_sh),\ - HUBP_SF(HUBP0_DCHUBP_REQ_SIZE_CONFIG, MPTE_GROUP_SIZE, mask_sh),\ HUBP_SF(HUBP0_DCHUBP_REQ_SIZE_CONFIG, SWATH_HEIGHT, mask_sh),\ HUBP_SF(HUBP0_DCHUBP_REQ_SIZE_CONFIG, PTE_ROW_HEIGHT_LINEAR, mask_sh),\ HUBP_SF(HUBP0_DCHUBP_REQ_SIZE_CONFIG_C, CHUNK_SIZE_C, mask_sh),\ @@ -339,7 +338,6 @@ HUBP_SF(HUBP0_DCHUBP_REQ_SIZE_CONFIG_C, META_CHUNK_SIZE_C, mask_sh),\ HUBP_SF(HUBP0_DCHUBP_REQ_SIZE_CONFIG_C, MIN_META_CHUNK_SIZE_C, mask_sh),\ HUBP_SF(HUBP0_DCHUBP_REQ_SIZE_CONFIG_C, DPTE_GROUP_SIZE_C, mask_sh),\ - HUBP_SF(HUBP0_DCHUBP_REQ_SIZE_CONFIG_C, MPTE_GROUP_SIZE_C, mask_sh),\ HUBP_SF(HUBP0_DCHUBP_REQ_SIZE_CONFIG_C, SWATH_HEIGHT_C, mask_sh),\ HUBP_SF(HUBP0_DCHUBP_REQ_SIZE_CONFIG_C, PTE_ROW_HEIGHT_LINEAR_C, mask_sh),\ HUBP_SF(HUBPREQ0_BLANK_OFFSET_0, REFCYC_H_BLANK_END, mask_sh),\ @@ -373,6 +371,11 @@ HUBP_SF(HUBPREQ0_DCN_SURF0_TTU_CNTL1, REFCYC_PER_REQ_DELIVERY_PRE, mask_sh),\ HUBP_SF(HUBP0_HUBP_CLK_CNTL, HUBP_CLOCK_ENABLE, mask_sh) +#define HUBP_MASK_SH_LIST_DCN(mask_sh)\ + HUBP_MASK_SH_LIST_DCN_COMMON(mask_sh),\ + HUBP_SF(HUBP0_DCHUBP_REQ_SIZE_CONFIG, MPTE_GROUP_SIZE, mask_sh),\ + HUBP_SF(HUBP0_DCHUBP_REQ_SIZE_CONFIG_C, MPTE_GROUP_SIZE_C, mask_sh) + /* Mask/shift struct generation macro for ASICs with VM */ #define HUBP_MASK_SH_LIST_DCN_VM(mask_sh)\ HUBP_SF(HUBPREQ0_NOM_PARAMETERS_0, DST_Y_PER_PTE_ROW_NOM_L, mask_sh),\ -- 2.17.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 08/20] drm/amd/display: Update plane scaling parameters for fast updates
From: Nicholas Kazlauskas [Why] Plane scaling parameters are not correctly filled or updated when performing fast updates. They're filled when creating the dc plane state and during atomic check. While the atomic check code path happens for the plane even during fast updates, the issue is that they're done in place on the dc_plane_state directly. This dc_plane_state may be the current state plane state being used by the hardware, so these parameters won't be correctly programmed. The new scaling parameters should instead be passed as an update to the plane. [How] Update fill_rects_from_plane_state to not modify dc_plane_state directly. Update the call sites that use this to fill in the appropriate values. Change-Id: I1db42853aeb469cd2a4dfa59d43fa46d710de71b Signed-off-by: Nicholas Kazlauskas Reviewed-by: Sun peng Li Acked-by: Bhawanpreet Lakha --- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 63 --- 1 file changed, 39 insertions(+), 24 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 fec4dcbaea7c..0cbfe2b52e79 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -2459,50 +2459,53 @@ static const struct drm_encoder_funcs amdgpu_dm_encoder_funcs = { }; static bool fill_rects_from_plane_state(const struct drm_plane_state *state, - struct dc_plane_state *plane_state) + struct rect *src_rect, + struct rect *dst_rect, + struct rect *clip_rect, + enum dc_rotation_angle *rotation) { - plane_state->src_rect.x = state->src_x >> 16; - plane_state->src_rect.y = state->src_y >> 16; + src_rect->x = state->src_x >> 16; + src_rect->y = state->src_y >> 16; /* we ignore the mantissa for now and do not deal with floating pixels :( */ - plane_state->src_rect.width = state->src_w >> 16; + src_rect->width = state->src_w >> 16; - if (plane_state->src_rect.width == 0) + if (src_rect->width == 0) return false; - plane_state->src_rect.height = state->src_h >> 16; - if (plane_state->src_rect.height == 0) + src_rect->height = state->src_h >> 16; + if (src_rect->height == 0) return false; - plane_state->dst_rect.x = state->crtc_x; - plane_state->dst_rect.y = state->crtc_y; + dst_rect->x = state->crtc_x; + dst_rect->y = state->crtc_y; if (state->crtc_w == 0) return false; - plane_state->dst_rect.width = state->crtc_w; + dst_rect->width = state->crtc_w; if (state->crtc_h == 0) return false; - plane_state->dst_rect.height = state->crtc_h; + dst_rect->height = state->crtc_h; - plane_state->clip_rect = plane_state->dst_rect; + *clip_rect = *dst_rect; switch (state->rotation & DRM_MODE_ROTATE_MASK) { case DRM_MODE_ROTATE_0: - plane_state->rotation = ROTATION_ANGLE_0; + *rotation = ROTATION_ANGLE_0; break; case DRM_MODE_ROTATE_90: - plane_state->rotation = ROTATION_ANGLE_90; + *rotation = ROTATION_ANGLE_90; break; case DRM_MODE_ROTATE_180: - plane_state->rotation = ROTATION_ANGLE_180; + *rotation = ROTATION_ANGLE_180; break; case DRM_MODE_ROTATE_270: - plane_state->rotation = ROTATION_ANGLE_270; + *rotation = ROTATION_ANGLE_270; break; default: - plane_state->rotation = ROTATION_ANGLE_0; + *rotation = ROTATION_ANGLE_0; break; } @@ -2882,7 +2885,11 @@ static int fill_plane_attributes(struct amdgpu_device *adev, const struct drm_crtc *crtc = plane_state->crtc; int ret = 0; - if (!fill_rects_from_plane_state(plane_state, dc_plane_state)) + if (!fill_rects_from_plane_state(plane_state, +&dc_plane_state->src_rect, +&dc_plane_state->dst_rect, +&dc_plane_state->clip_rect, +&dc_plane_state->rotation)) return -EINVAL; ret = fill_plane_attributes_from_fb( @@ -4114,12 +4121,17 @@ static int dm_plane_atomic_check(struct drm_plane *plane, { struct amdgpu_device *adev = plane->dev->dev_private; struct dc *dc = adev->dm.dc; - struct dm_plane_state *dm_plane_state = to_dm_plane_state(state); + struct dm_plane_state *dm_plane_state; + struct rect src_rect, dst_rect, clip_rect; + enum dc_rotation_angle rotation; + + d
[PATCH 10/20] drm/amd/display: If one stream full updates, full update all planes
From: David Francis [Why] On some compositors, with two monitors attached, VT terminal switch can cause a graphical issue by the following means: There are two streams, one for each monitor. Each stream has one plane current state: M1:S1->P1 M2:S2->P2 The user calls for a terminal switch and a commit is made to change both planes to linear swizzle mode. In atomic check, a new dc_state is constructed with new planes on each stream new state: M1:S1->P3 M2:S2->P4 In commit tail, each stream is committed, one at a time. The first stream (S1) updates properly, triggerring a full update and replacing the state current state: M1:S1->P3 M2:S2->P4 The update for S2 comes in, but dc detects that there is no difference between the stream and plane in the new and current states, and so triggers a fast update. The fast update does not program swizzle, so the second monitor is corrupted [How] Add a flag to dc_plane_state that forces full updates When a stream undergoes a full update, set this flag on all changed planes, then clear it on the current stream Subsequent streams will get full updates as a result Change-Id: I211b58776bfc3b84ed1cfa3ea11b9975101a44a8 Signed-off-by: David Francis Signed-off-by: Nicholas Kazlauskas Reviewed-by: Roman Li Acked-by: Bhawanpreet Lakha Acked-by: Nicholas Kazlauskas --- drivers/gpu/drm/amd/display/dc/core/dc.c | 19 +++ drivers/gpu/drm/amd/display/dc/dc.h | 3 +++ 2 files changed, 22 insertions(+) diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c b/drivers/gpu/drm/amd/display/dc/core/dc.c index ceeacd727f5a..cc1dcb486bb8 100644 --- a/drivers/gpu/drm/amd/display/dc/core/dc.c +++ b/drivers/gpu/drm/amd/display/dc/core/dc.c @@ -1476,6 +1476,11 @@ static enum surface_update_type det_surface_update(const struct dc *dc, return UPDATE_TYPE_FULL; } + if (u->surface->force_full_update) { + update_flags->bits.full_update = 1; + return UPDATE_TYPE_FULL; + } + type = get_plane_info_update_type(u); elevate_update_type(&overall_type, type); @@ -1911,6 +1916,14 @@ void dc_commit_updates_for_stream(struct dc *dc, } dc_resource_state_copy_construct(state, context); + + for (i = 0; i < dc->res_pool->pipe_count; i++) { + struct pipe_ctx *new_pipe = &context->res_ctx.pipe_ctx[i]; + struct pipe_ctx *old_pipe = &dc->current_state->res_ctx.pipe_ctx[i]; + + if (new_pipe->plane_state && new_pipe->plane_state != old_pipe->plane_state) + new_pipe->plane_state->force_full_update = true; + } } @@ -1947,6 +1960,12 @@ void dc_commit_updates_for_stream(struct dc *dc, dc->current_state = context; dc_release_state(old); + for (i = 0; i < dc->res_pool->pipe_count; i++) { + struct pipe_ctx *pipe_ctx = &context->res_ctx.pipe_ctx[i]; + + if (pipe_ctx->plane_state && pipe_ctx->stream == stream) + pipe_ctx->plane_state->force_full_update = false; + } } /*let's use current_state to update watermark etc*/ if (update_type >= UPDATE_TYPE_FULL) diff --git a/drivers/gpu/drm/amd/display/dc/dc.h b/drivers/gpu/drm/amd/display/dc/dc.h index b0fdf8387e7a..4222ffed11ff 100644 --- a/drivers/gpu/drm/amd/display/dc/dc.h +++ b/drivers/gpu/drm/amd/display/dc/dc.h @@ -573,6 +573,9 @@ struct dc_plane_state { struct dc_plane_status status; struct dc_context *ctx; + /* HACK: Workaround for forcing full reprogramming under some conditions */ + bool force_full_update; + /* private to dc_surface.c */ enum dc_irq_source irq_source; struct kref refcount; -- 2.17.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 11/20] drm/amd/display: Recalculate pitch when buffers change
From: Nicholas Kazlauskas [Why] Pitch was only calculated based on format whenever the plane state was recreated. This could result in surface corruption due to the incorrect pitch being programmed when the surface pitch changed during commits where state->allow_modeset = false. [How] Recalculate pitch at the same time we update the buffer address and other buffer attributes. This function was previously called fill_plane_tiling_attributes but I've also renamed it to fill_plane_buffer_attributes to clarify the actual intent of the function now that it's handling most buffer related attributes. Change-Id: I2f7a65165a30339a9115c18fa06397260e74c252 Signed-off-by: Nicholas Kazlauskas Reviewed-by: David Francis Acked-by: Bhawanpreet Lakha --- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 73 +-- 1 file changed, 34 insertions(+), 39 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 9b8546c7e947..ee4738636aa0 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -2601,28 +2601,53 @@ static int fill_plane_dcc_attributes(struct amdgpu_device *adev, } static int -fill_plane_tiling_attributes(struct amdgpu_device *adev, +fill_plane_buffer_attributes(struct amdgpu_device *adev, const struct amdgpu_framebuffer *afb, const struct dc_plane_state *plane_state, union dc_tiling_info *tiling_info, +union plane_size *plane_size, struct dc_plane_dcc_param *dcc, struct dc_plane_address *address, uint64_t tiling_flags) { + const struct drm_framebuffer *fb = &afb->base; int ret; memset(tiling_info, 0, sizeof(*tiling_info)); + memset(plane_size, 0, sizeof(*plane_size)); memset(dcc, 0, sizeof(*dcc)); memset(address, 0, sizeof(*address)); if (plane_state->format < SURFACE_PIXEL_FORMAT_VIDEO_BEGIN) { + plane_size->grph.surface_size.x = 0; + plane_size->grph.surface_size.y = 0; + plane_size->grph.surface_size.width = fb->width; + plane_size->grph.surface_size.height = fb->height; + plane_size->grph.surface_pitch = + fb->pitches[0] / fb->format->cpp[0]; + address->type = PLN_ADDR_TYPE_GRAPHICS; address->grph.addr.low_part = lower_32_bits(afb->address); address->grph.addr.high_part = upper_32_bits(afb->address); } else { - const struct drm_framebuffer *fb = &afb->base; uint64_t chroma_addr = afb->address + fb->offsets[1]; + plane_size->video.luma_size.x = 0; + plane_size->video.luma_size.y = 0; + plane_size->video.luma_size.width = fb->width; + plane_size->video.luma_size.height = fb->height; + plane_size->video.luma_pitch = + fb->pitches[0] / fb->format->cpp[0]; + + plane_size->video.chroma_size.x = 0; + plane_size->video.chroma_size.y = 0; + /* TODO: set these based on surface format */ + plane_size->video.chroma_size.width = fb->width / 2; + plane_size->video.chroma_size.height = fb->height / 2; + + plane_size->video.chroma_pitch = + fb->pitches[1] / fb->format->cpp[1]; + address->type = PLN_ADDR_TYPE_VIDEO_PROGRESSIVE; address->video_progressive.luma_addr.low_part = lower_32_bits(afb->address); @@ -2743,41 +2768,9 @@ static int fill_plane_attributes_from_fb(struct amdgpu_device *adev, return -EINVAL; } - memset(&plane_state->address, 0, sizeof(plane_state->address)); - - if (plane_state->format < SURFACE_PIXEL_FORMAT_VIDEO_BEGIN) { - plane_state->plane_size.grph.surface_size.x = 0; - plane_state->plane_size.grph.surface_size.y = 0; - plane_state->plane_size.grph.surface_size.width = fb->width; - plane_state->plane_size.grph.surface_size.height = fb->height; - plane_state->plane_size.grph.surface_pitch = - fb->pitches[0] / fb->format->cpp[0]; - /* TODO: unhardcode */ - plane_state->color_space = COLOR_SPACE_SRGB; - - } else { - plane_state->plane_size.video.luma_size.x = 0; - plane_state->plane_size.video.luma_size.y = 0; - plane_state->plane_size.video.luma_size.width = fb->width; - plane_state->plane_size.video.luma_size.height = fb->height; - plane_state->plane_size.video.luma_pitch = - fb->pitches[0] / fb->fo
[PATCH 09/20] drm/amd/display: Maintain z-ordering when creating planes
From: Nicholas Kazlauskas [Why] The overlay will be incorrectly placed *below* the primary plane for commits with state->allow_modeset = true because the primary plane won't be removed and recreated in the same commit. [How] Add the should_reset_plane helper to determine if the plane should be reset or not. If we need to add or force reset any plane in the context then we'll need to do the same for every plane on the stream. Unfortunately we need to do the remove / recreate routine for removing planes as well since DC currently isn't well equipped to handle the plane with the top pipe being removed with other planes still active. Change-Id: I62529bcb14fe820addda68ce6e5f0a4a4b001d56 Signed-off-by: Nicholas Kazlauskas Reviewed-by: Sun peng Li Acked-by: Bhawanpreet Lakha --- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 74 +-- 1 file changed, 69 insertions(+), 5 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 0cbfe2b52e79..9b8546c7e947 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -6157,6 +6157,69 @@ static int dm_update_crtc_state(struct amdgpu_display_manager *dm, return ret; } +static bool should_reset_plane(struct drm_atomic_state *state, + struct drm_plane *plane, + struct drm_plane_state *old_plane_state, + struct drm_plane_state *new_plane_state) +{ + struct drm_plane *other; + struct drm_plane_state *old_other_state, *new_other_state; + struct drm_crtc_state *new_crtc_state; + int i; + + /* +* TODO: Remove this hack once the checks below are sufficient +* enough to determine when we need to reset all the planes on +* the stream. +*/ + if (state->allow_modeset) + return true; + + /* Exit early if we know that we're adding or removing the plane. */ + if (old_plane_state->crtc != new_plane_state->crtc) + return true; + + /* old crtc == new_crtc == NULL, plane not in context. */ + if (!new_plane_state->crtc) + return false; + + new_crtc_state = + drm_atomic_get_new_crtc_state(state, new_plane_state->crtc); + + if (!new_crtc_state) + return true; + + if (drm_atomic_crtc_needs_modeset(new_crtc_state)) + return true; + + /* +* If there are any new primary or overlay planes being added or +* removed then the z-order can potentially change. To ensure +* correct z-order and pipe acquisition the current DC architecture +* requires us to remove and recreate all existing planes. +* +* TODO: Come up with a more elegant solution for this. +*/ + for_each_oldnew_plane_in_state(state, other, old_other_state, new_other_state, i) { + if (other->type == DRM_PLANE_TYPE_CURSOR) + continue; + + if (old_other_state->crtc != new_plane_state->crtc && + new_other_state->crtc != new_plane_state->crtc) + continue; + + if (old_other_state->crtc != new_other_state->crtc) + return true; + + /* TODO: Remove this once we can handle fast format changes. */ + if (old_other_state->fb && new_other_state->fb && + old_other_state->fb->format != new_other_state->fb->format) + return true; + } + + return false; +} + static int dm_update_plane_state(struct dc *dc, struct drm_atomic_state *state, struct drm_plane *plane, @@ -6171,8 +6234,7 @@ static int dm_update_plane_state(struct dc *dc, struct drm_crtc_state *old_crtc_state, *new_crtc_state; struct dm_crtc_state *dm_new_crtc_state, *dm_old_crtc_state; struct dm_plane_state *dm_new_plane_state, *dm_old_plane_state; - /* TODO return page_flip_needed() function */ - bool pflip_needed = !state->allow_modeset; + bool needs_reset; int ret = 0; @@ -6185,10 +6247,12 @@ static int dm_update_plane_state(struct dc *dc, if (plane->type == DRM_PLANE_TYPE_CURSOR) return 0; + needs_reset = should_reset_plane(state, plane, old_plane_state, +new_plane_state); + /* Remove any changed/removed planes */ if (!enable) { - if (pflip_needed && - plane->type != DRM_PLANE_TYPE_OVERLAY) + if (!needs_reset) return 0; if (!old_plane_crtc) @@ -6239,7 +6303,7 @@ static int dm_update_plane_state(struct dc *dc, if (!dm_new_crtc_state->stream) retur
[PATCH 13/20] drm/amd/display: change name from dc_link_get_verified_link_cap to dc_link_get_link_cap
From: Samson Tam [Why] DM doesn't need to know which link cap is being retrieved ( verified or preferred ). Let DC figure it out. [How] Change name. Change-Id: I5585ac7dcdf58216b2942f7ab6758c5d15719d0d Signed-off-by: Samson Tam Reviewed-by: Tony Cheng Acked-by: Bhawanpreet Lakha --- drivers/gpu/drm/amd/display/dc/core/dc.c | 2 +- drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c | 3 +-- drivers/gpu/drm/amd/display/dc/dc_link.h | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c b/drivers/gpu/drm/amd/display/dc/core/dc.c index cc1dcb486bb8..71a13eb733b9 100644 --- a/drivers/gpu/drm/amd/display/dc/core/dc.c +++ b/drivers/gpu/drm/amd/display/dc/core/dc.c @@ -603,7 +603,7 @@ uint32_t dc_link_bandwidth_kbps( } -const struct dc_link_settings *dc_link_get_verified_link_cap( +const struct dc_link_settings *dc_link_get_link_cap( const struct dc_link *link) { if (link->preferred_link_setting.lane_count != LANE_COUNT_UNKNOWN && diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c index ba7502f3d0eb..a6424c70f4c5 100644 --- a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c @@ -1548,8 +1548,7 @@ bool dp_validate_mode_timing( timing->v_addressable == (uint32_t) 480) return true; - /* We always use verified link settings */ - link_setting = dc_link_get_verified_link_cap(link); + link_setting = dc_link_get_link_cap(link); /* TODO: DYNAMIC_VALIDATION needs to be implemented */ /*if (flags.DYNAMIC_VALIDATION == 1 && diff --git a/drivers/gpu/drm/amd/display/dc/dc_link.h b/drivers/gpu/drm/amd/display/dc/dc_link.h index 4e26d6e93b31..cc7ffac64c96 100644 --- a/drivers/gpu/drm/amd/display/dc/dc_link.h +++ b/drivers/gpu/drm/amd/display/dc/dc_link.h @@ -250,7 +250,7 @@ uint32_t dc_link_bandwidth_kbps( const struct dc_link *link, const struct dc_link_settings *link_setting); -const struct dc_link_settings *dc_link_get_verified_link_cap( +const struct dc_link_settings *dc_link_get_link_cap( const struct dc_link *link); bool dc_submit_i2c( -- 2.17.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 06/20] drm/amd/display: Read eDP link settings on detection
From: Anthony Koo [Why] Unlike external DP panels, internal eDP does not perform verify link caps because the panel connection is fixed. So if GOP enabled the eDP at boot, we can retain its trained link settings to optimize. [How] Read the lane count and link rate by reading this information from DPCD 100h, 101h, 115h Change-Id: If2f9588401731237e4dd7c587f4217bca4e5e08e Signed-off-by: Anthony Koo Reviewed-by: Aric Cyr Acked-by: Bhawanpreet Lakha --- .../gpu/drm/amd/display/dc/core/dc_link_dp.c | 30 +++ 1 file changed, 30 insertions(+) diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c index acb4f829e042..ba7502f3d0eb 100644 --- a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c @@ -2587,6 +2587,9 @@ void detect_edp_sink_caps(struct dc_link *link) uint32_t entry; uint32_t link_rate_in_khz; enum dc_link_rate link_rate = LINK_RATE_UNKNOWN; + union lane_count_set lane_count_set = { {0} }; + uint8_t link_bw_set; + uint8_t link_rate_set; retrieve_link_cap(link); link->dpcd_caps.edp_supported_link_rates_count = 0; @@ -2612,6 +2615,33 @@ void detect_edp_sink_caps(struct dc_link *link) } } link->verified_link_cap = link->reported_link_cap; + + // Read DPCD 00101h to find out the number of lanes currently set + core_link_read_dpcd(link, DP_LANE_COUNT_SET, + &lane_count_set.raw, sizeof(lane_count_set)); + link->cur_link_settings.lane_count = lane_count_set.bits.LANE_COUNT_SET; + + // Read DPCD 00100h to find if standard link rates are set + core_link_read_dpcd(link, DP_LINK_BW_SET, + &link_bw_set, sizeof(link_bw_set)); + + if (link_bw_set == 0) { + /* If standard link rates are not being used, +* Read DPCD 00115h to find the link rate set used +*/ + core_link_read_dpcd(link, DP_LINK_RATE_SET, + &link_rate_set, sizeof(link_rate_set)); + + if (link_rate_set < link->dpcd_caps.edp_supported_link_rates_count) { + link->cur_link_settings.link_rate = + link->dpcd_caps.edp_supported_link_rates[link_rate_set]; + link->cur_link_settings.link_rate_set = link_rate_set; + link->cur_link_settings.use_link_rate_set = true; + } + } else { + link->cur_link_settings.link_rate = link_bw_set; + link->cur_link_settings.use_link_rate_set = false; + } } void dc_link_dp_enable_hpd(const struct dc_link *link) -- 2.17.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 02/20] drm/amd/display: Add DRM color properties for primary planes
From: Nicholas Kazlauskas [Why] We need DC's color space to match the color encoding and color space specified by userspace to correctly render YUV surfaces. [How] Add the DRM color properties when the DC plane supports NV12. Change-Id: Ie3eb9800a9a7954d05f691b277e7ca5a25164d5d Signed-off-by: Nicholas Kazlauskas Reviewed-by: Sun peng Li Acked-by: Bhawanpreet Lakha --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 12 1 file changed, 12 insertions(+) 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 6e922099f616..9a1ef3df717e 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -4281,6 +4281,18 @@ static int amdgpu_dm_plane_init(struct amdgpu_display_manager *dm, drm_plane_create_blend_mode_property(plane, blend_caps); } + if (plane->type == DRM_PLANE_TYPE_PRIMARY && + plane_cap && plane_cap->supports_nv12) { + /* This only affects YUV formats. */ + drm_plane_create_color_properties( + plane, + BIT(DRM_COLOR_YCBCR_BT601) | + BIT(DRM_COLOR_YCBCR_BT709), + BIT(DRM_COLOR_YCBCR_LIMITED_RANGE) | + BIT(DRM_COLOR_YCBCR_FULL_RANGE), + DRM_COLOR_YCBCR_BT709, DRM_COLOR_YCBCR_LIMITED_RANGE); + } + drm_plane_helper_add(plane, &dm_plane_helper_funcs); /* Create (reset) the plane state */ -- 2.17.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 03/20] drm/amd/display: expand plane caps to include fp16 and scaling capability
From: Jun Lei [why] there are some scaling capabilities such as fp16 which are known to be unsupported on a given ASIC. exposing these static capabilities allows much simpler implementation for OS interfaces which require to report such static capabilities to reduce the number of dynamic validation calls [how] refactor the existing plane caps to be more extensible, and add fp16 and scaling capabilities Change-Id: I8feb15d433f21a10df26ef3cd07cc892de34ce4b Signed-off-by: Jun Lei Reviewed-by: Tony Cheng Acked-by: Bhawanpreet Lakha Acked-by: Harry Wentland --- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 6 +-- drivers/gpu/drm/amd/display/dc/dc.h | 23 ++- .../amd/display/dc/dce100/dce100_resource.c | 19 +- .../amd/display/dc/dce110/dce110_resource.c | 38 ++- .../amd/display/dc/dce112/dce112_resource.c | 19 +- .../amd/display/dc/dce120/dce120_resource.c | 19 +- .../drm/amd/display/dc/dce80/dce80_resource.c | 19 +- .../drm/amd/display/dc/dcn10/dcn10_resource.c | 20 +- 8 files changed, 150 insertions(+), 13 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 9a1ef3df717e..fec4dcbaea7c 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -2085,7 +2085,7 @@ static int amdgpu_dm_initialize_drm_device(struct amdgpu_device *adev) if (!plane->blends_with_above || !plane->blends_with_below) continue; - if (!plane->supports_argb) + if (!plane->pixel_format_support.argb) continue; if (initialize_plane(dm, NULL, primary_planes + i, @@ -4228,7 +4228,7 @@ static int get_plane_formats(const struct drm_plane *plane, formats[num_formats++] = rgb_formats[i]; } - if (plane_cap && plane_cap->supports_nv12) + if (plane_cap && plane_cap->pixel_format_support.nv12) formats[num_formats++] = DRM_FORMAT_NV12; break; @@ -4282,7 +4282,7 @@ static int amdgpu_dm_plane_init(struct amdgpu_display_manager *dm, } if (plane->type == DRM_PLANE_TYPE_PRIMARY && - plane_cap && plane_cap->supports_nv12) { + plane_cap && plane_cap->pixel_format_support.nv12) { /* This only affects YUV formats. */ drm_plane_create_color_properties( plane, diff --git a/drivers/gpu/drm/amd/display/dc/dc.h b/drivers/gpu/drm/amd/display/dc/dc.h index ac2506616025..637a7b39eea4 100644 --- a/drivers/gpu/drm/amd/display/dc/dc.h +++ b/drivers/gpu/drm/amd/display/dc/dc.h @@ -69,8 +69,27 @@ struct dc_plane_cap { uint32_t blends_with_above : 1; uint32_t blends_with_below : 1; uint32_t per_pixel_alpha : 1; - uint32_t supports_argb : 1; - uint32_t supports_nv12 : 1; + struct { + uint32_t argb : 1; + uint32_t nv12 : 1; + uint32_t fp16 : 1; + } pixel_format_support; + // max upscaling factor x1000 + // upscaling factors are always >= 1 + // for example, 1080p -> 8K is 4.0, or 4000 raw value + struct { + uint32_t argb; + uint32_t nv12; + uint32_t fp16; + } max_upscale_factor; + // max downscale factor x1000 + // downscale factors are always <= 1 + // for example, 8K -> 1080p is 0.25, or 250 raw value + struct { + uint32_t argb; + uint32_t nv12; + uint32_t fp16; + } max_downscale_factor; }; struct dc_caps { diff --git a/drivers/gpu/drm/amd/display/dc/dce100/dce100_resource.c b/drivers/gpu/drm/amd/display/dc/dce100/dce100_resource.c index 767d37c6d942..f38ea29b3377 100644 --- a/drivers/gpu/drm/amd/display/dc/dce100/dce100_resource.c +++ b/drivers/gpu/drm/amd/display/dc/dce100/dce100_resource.c @@ -380,7 +380,24 @@ static const struct resource_caps res_cap = { static const struct dc_plane_cap plane_cap = { .type = DC_PLANE_TYPE_DCE_RGB, - .supports_argb = true, + + .pixel_format_support = { + .argb = true, + .nv12 = false, + .fp16 = false + }, + + .max_upscale_factor = { + .argb = 16000, + .nv12 = 1, + .fp16 = 1 + }, + + .max_downscale_factor = { + .argb = 250, + .nv12 = 1, + .fp16 = 1 + } }; #define CTX ctx diff --git a/drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c b/drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c index 7c4914b2b524..d5ebc4538711 100644 --- a/drivers/gpu/drm
[PATCH 01/20] drm/amd/display: Expose support for NV12 on suitable planes
From: Nicholas Kazlauskas [Why] Hardware can support video surfaces and DC tells us which planes are suitable via DC plane caps. [How] The supported formats array will now vary based on what DC tells us, so create an array and fill it dynamically based on plane types and caps. Ideally we'd query support for every format via DC plane caps, but for the framework is in place to do so later with this. Change-Id: Ic80f12d3f7e95fc33219f90e95219468f044ac6d Signed-off-by: Nicholas Kazlauskas Reviewed-by: Sun peng Li Acked-by: Bhawanpreet Lakha --- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 86 --- 1 file changed, 55 insertions(+), 31 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 511346750390..6e922099f616 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -4207,46 +4207,71 @@ static const u32 cursor_formats[] = { DRM_FORMAT_ARGB }; -static int amdgpu_dm_plane_init(struct amdgpu_display_manager *dm, - struct drm_plane *plane, - unsigned long possible_crtcs, - const struct dc_plane_cap *plane_cap) +static int get_plane_formats(const struct drm_plane *plane, +const struct dc_plane_cap *plane_cap, +uint32_t *formats, int max_formats) { - int res = -EPERM; + int i, num_formats = 0; + + /* +* TODO: Query support for each group of formats directly from +* DC plane caps. This will require adding more formats to the +* caps list. +*/ switch (plane->type) { case DRM_PLANE_TYPE_PRIMARY: - res = drm_universal_plane_init( - dm->adev->ddev, - plane, - possible_crtcs, - &dm_plane_funcs, - rgb_formats, - ARRAY_SIZE(rgb_formats), - NULL, plane->type, NULL); + for (i = 0; i < ARRAY_SIZE(rgb_formats); ++i) { + if (num_formats >= max_formats) + break; + + formats[num_formats++] = rgb_formats[i]; + } + + if (plane_cap && plane_cap->supports_nv12) + formats[num_formats++] = DRM_FORMAT_NV12; break; + case DRM_PLANE_TYPE_OVERLAY: - res = drm_universal_plane_init( - dm->adev->ddev, - plane, - possible_crtcs, - &dm_plane_funcs, - overlay_formats, - ARRAY_SIZE(overlay_formats), - NULL, plane->type, NULL); + for (i = 0; i < ARRAY_SIZE(overlay_formats); ++i) { + if (num_formats >= max_formats) + break; + + formats[num_formats++] = overlay_formats[i]; + } break; + case DRM_PLANE_TYPE_CURSOR: - res = drm_universal_plane_init( - dm->adev->ddev, - plane, - possible_crtcs, - &dm_plane_funcs, - cursor_formats, - ARRAY_SIZE(cursor_formats), - NULL, plane->type, NULL); + for (i = 0; i < ARRAY_SIZE(cursor_formats); ++i) { + if (num_formats >= max_formats) + break; + + formats[num_formats++] = cursor_formats[i]; + } break; } + return num_formats; +} + +static int amdgpu_dm_plane_init(struct amdgpu_display_manager *dm, + struct drm_plane *plane, + unsigned long possible_crtcs, + const struct dc_plane_cap *plane_cap) +{ + uint32_t formats[32]; + int num_formats; + int res = -EPERM; + + num_formats = get_plane_formats(plane, plane_cap, formats, + ARRAY_SIZE(formats)); + + res = drm_universal_plane_init(dm->adev->ddev, plane, possible_crtcs, + &dm_plane_funcs, formats, num_formats, + NULL, plane->type, NULL); + if (res) + return res; + if (plane->type == DRM_PLANE_TYPE_OVERLAY && plane_cap && plane_cap->per_pixel_alpha) { unsigned int blend_caps = BIT(DRM_MODE_BLEND_PIXEL_NONE) | @@ -4262
[PATCH 14/20] drm/amd/display: Add basic downscale and upscale valdiation
From: Nicholas Kazlauskas [Why] Planes have downscaling limits and upscaling limits per format and DM is expected to validate these using DC caps. We should fail atomic check validation if we aren't capable of doing the scaling. [How] We don't currently create store which DC plane maps to which DRM plane so we can't easily check the caps directly. For now add basic constraints that cover the absolute min and max downscale / upscale limits for most RGB and YUV formats across ASICs. Leave a TODO indicating that these should really be done with DC caps. We'll probably need to subclass DRM planes again in order to correctly identify which DC plane maps to it. Change-Id: I18524d0e5d8a96ae3ae0e5d0eb3906092201a125 Signed-off-by: Nicholas Kazlauskas Reviewed-by: Harry Wentland Reviewed-by: Tony Cheng Acked-by: Bhawanpreet Lakha --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 15 +++ 1 file changed, 15 insertions(+) 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 2e5689412871..f8aaf9a1b6bb 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -2462,6 +2462,8 @@ static const struct drm_encoder_funcs amdgpu_dm_encoder_funcs = { static int fill_dc_scaling_info(const struct drm_plane_state *state, struct dc_scaling_info *scaling_info) { + int scale_w, scale_h; + memset(scaling_info, 0, sizeof(*scaling_info)); /* Source is fixed 16.16 but we ignore mantissa for now... */ @@ -2492,6 +2494,19 @@ static int fill_dc_scaling_info(const struct drm_plane_state *state, /* DRM doesn't specify clipping on destination output. */ scaling_info->clip_rect = scaling_info->dst_rect; + /* TODO: Validate scaling per-format with DC plane caps */ + scale_w = scaling_info->dst_rect.width * 1000 / + scaling_info->src_rect.width; + + if (scale_w < 250 || scale_w > 16000) + return -EINVAL; + + scale_h = scaling_info->dst_rect.height * 1000 / + scaling_info->src_rect.height; + + if (scale_h < 250 || scale_h > 16000) + return -EINVAL; + /* * The "scaling_quality" can be ignored for now, quality = 0 has DC * assume reasonable defaults based on the format. -- 2.17.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 15/20] drm/amd/display: Use surface directly when checking update type
From: Nicholas Kazlauskas [Why] DC expects the surface memory address to identify the surface. This doesn't work with what we're doing with the temporary surfaces, it will always assume this is a full update because the surface isn't in the current context. [How] Use the surface directly. This doesn't give us much improvement yet, since we always create a new dc_plane_state when state->allow_modeset is true. The call into dc_check_update_surfaces_for_stream also needs to be locked, for two reasons: 1. It checks the current DC state 2. It modifies the surface update flags Both of which could be currently in the middle of commit work from commit tail. A TODO here is to pass the context explicitly into this function and find a way to get the surface update flags out of it without modifying the surface in place. Change-Id: Icd942bae4da7e16070a68c00dcf44908d195b63a Signed-off-by: Nicholas Kazlauskas Reviewed-by: David Francis Acked-by: Bhawanpreet Lakha --- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 28 --- 1 file changed, 12 insertions(+), 16 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 f8aaf9a1b6bb..d87d80aee662 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -6375,10 +6375,11 @@ static int dm_update_plane_state(struct dc *dc, } static int -dm_determine_update_type_for_commit(struct dc *dc, +dm_determine_update_type_for_commit(struct amdgpu_display_manager *dm, struct drm_atomic_state *state, enum surface_update_type *out_type) { + struct dc *dc = dm->dc; struct dm_atomic_state *dm_state = NULL, *old_dm_state = NULL; int i, j, num_plane, ret = 0; struct drm_plane_state *old_plane_state, *new_plane_state; @@ -6392,14 +6393,12 @@ dm_determine_update_type_for_commit(struct dc *dc, struct dc_stream_status *status = NULL; struct dc_surface_update *updates; - struct dc_plane_state *surface; enum surface_update_type update_type = UPDATE_TYPE_FAST; updates = kcalloc(MAX_SURFACES, sizeof(*updates), GFP_KERNEL); - surface = kcalloc(MAX_SURFACES, sizeof(*surface), GFP_KERNEL); - if (!updates || !surface) { - DRM_ERROR("Plane or surface update failed to allocate"); + if (!updates) { + DRM_ERROR("Failed to allocate plane updates\n"); /* Set type to FULL to avoid crashing in DC*/ update_type = UPDATE_TYPE_FULL; goto cleanup; @@ -6440,17 +6439,9 @@ dm_determine_update_type_for_commit(struct dc *dc, if (crtc != new_plane_crtc) continue; - updates[num_plane].surface = &surface[num_plane]; + updates[num_plane].surface = new_dm_plane_state->dc_state; if (new_crtc_state->mode_changed) { - updates[num_plane].surface->src_rect = - new_dm_plane_state->dc_state->src_rect; - updates[num_plane].surface->dst_rect = - new_dm_plane_state->dc_state->dst_rect; - updates[num_plane].surface->rotation = - new_dm_plane_state->dc_state->rotation; - updates[num_plane].surface->in_transfer_func = - new_dm_plane_state->dc_state->in_transfer_func; stream_update.dst = new_dm_crtc_state->stream->dst; stream_update.src = new_dm_crtc_state->stream->src; } @@ -6485,8 +6476,14 @@ dm_determine_update_type_for_commit(struct dc *dc, status = dc_stream_get_status_from_state(old_dm_state->context, new_dm_crtc_state->stream); + /* +* TODO: DC modifies the surface during this call so we need +* to lock here - find a way to do this without locking. +*/ + mutex_lock(&dm->dc_lock); update_type = dc_check_update_surfaces_for_stream(dc, updates, num_plane, &stream_update, status); + mutex_unlock(&dm->dc_lock); if (update_type > UPDATE_TYPE_MED) { update_type = UPDATE_TYPE_FULL; @@ -6496,7 +6493,6 @@ dm_determine_update_type_for_commit(struct dc *dc, cleanup: kfree(updates); - kfree(surface); *out_type = update_type; return ret; @@ -6680,7 +6676,7 @@ static int amdgpu_dm_atomic_check(struct drm
[PATCH 18/20] drm/amd/display: Relax requirements for CRTCs to be enabled
From: Nicholas Kazlauskas [Why] As long as we have at least one non-cursor plane enabled on a CRTC then the CRTC itself can remain enabled. This will allow for commits where there's an overlay plane enabled but no primary plane enabled. [How] Remove existing primary plane fb != NULL checks and replace them with the new does_crtc_have_active_plane helper. This will be called from atomic check when validating the CRTC. Since the primary plane state can now potentially be NULL we'll need to guard for that when accessing it in some of the cursor logic. Change-Id: I25f32ff13d1e81e20eea233ffdddadf704ecd8d2 Signed-off-by: Nicholas Kazlauskas Reviewed-by: David Francis Acked-by: Bhawanpreet Lakha --- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 56 +++ 1 file changed, 44 insertions(+), 12 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 3ff87bc31a8d..6aef4a3574db 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -3948,6 +3948,38 @@ static void dm_crtc_helper_disable(struct drm_crtc *crtc) { } +static bool does_crtc_have_active_plane(struct drm_crtc_state *new_crtc_state) +{ + struct drm_atomic_state *state = new_crtc_state->state; + struct drm_plane *plane; + int num_active = 0; + + drm_for_each_plane_mask(plane, state->dev, new_crtc_state->plane_mask) { + struct drm_plane_state *new_plane_state; + + /* Cursor planes are "fake". */ + if (plane->type == DRM_PLANE_TYPE_CURSOR) + continue; + + new_plane_state = drm_atomic_get_new_plane_state(state, plane); + + if (!new_plane_state) { + /* +* The plane is enable on the CRTC and hasn't changed +* state. This means that it previously passed +* validation and is therefore enabled. +*/ + num_active += 1; + continue; + } + + /* We need a framebuffer to be considered enabled. */ + num_active += (new_plane_state->fb != NULL); + } + + return num_active > 0; +} + static int dm_crtc_helper_atomic_check(struct drm_crtc *crtc, struct drm_crtc_state *state) { @@ -3966,6 +3998,11 @@ static int dm_crtc_helper_atomic_check(struct drm_crtc *crtc, if (!dm_crtc_state->stream) return 0; + /* We want at least one hardware plane enabled to use the stream. */ + if (state->enable && state->active && + !does_crtc_have_active_plane(state)) + return -EINVAL; + if (dc_validate_stream(dc, dm_crtc_state->stream) == DC_OK) return 0; @@ -4939,9 +4976,13 @@ static int get_cursor_position(struct drm_plane *plane, struct drm_crtc *crtc, x = plane->state->crtc_x; y = plane->state->crtc_y; - /* avivo cursor are offset into the total surface */ - x += crtc->primary->state->src_x >> 16; - y += crtc->primary->state->src_y >> 16; + + if (crtc->primary->state) { + /* avivo cursor are offset into the total surface */ + x += crtc->primary->state->src_x >> 16; + y += crtc->primary->state->src_y >> 16; + } + if (x < 0) { xorigin = min(-x, amdgpu_crtc->max_cursor_width - 1); x = 0; @@ -5965,21 +6006,12 @@ static int dm_update_crtc_state(struct amdgpu_display_manager *dm, struct amdgpu_dm_connector *aconnector = NULL; struct drm_connector_state *drm_new_conn_state = NULL, *drm_old_conn_state = NULL; struct dm_connector_state *dm_new_conn_state = NULL, *dm_old_conn_state = NULL; - struct drm_plane_state *new_plane_state = NULL; new_stream = NULL; dm_old_crtc_state = to_dm_crtc_state(old_crtc_state); dm_new_crtc_state = to_dm_crtc_state(new_crtc_state); acrtc = to_amdgpu_crtc(crtc); - - new_plane_state = drm_atomic_get_new_plane_state(state, new_crtc_state->crtc->primary); - - if (new_crtc_state->enable && new_plane_state && !new_plane_state->fb) { - ret = -EINVAL; - goto fail; - } - aconnector = amdgpu_dm_find_first_crtc_matching_connector(state, crtc); /* TODO This hack should go away */ -- 2.17.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 16/20] drm/amd/display: Don't warn when DC update type > DM guess
From: Nicholas Kazlauskas [Why] DM thinks that the update type should be full whenever a stream or plane is added or removed (including recreations). This won't match in the case where DC thinks what looks like a fast update to DM is actually a medium or full - like scaling changes that affect bandwidth and clocks. [How] Drop this warning. DC knows better than the DM does for determining cases like this. The other warning can be kept for now since it would warn on a pretty serious DC or DM bug. Change-Id: I3eaf72b3abde6ca6dcfe72f0f734fe5e215798dd Signed-off-by: Nicholas Kazlauskas Reviewed-by: David Francis Acked-by: Bhawanpreet Lakha --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 --- 1 file changed, 3 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 d87d80aee662..3aa1b9c05f88 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -6691,9 +6691,6 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev, */ if (lock_and_validation_needed && overall_update_type <= UPDATE_TYPE_FAST) WARN(1, "Global lock should be Set, overall_update_type should be UPDATE_TYPE_MED or UPDATE_TYPE_FULL"); - else if (!lock_and_validation_needed && overall_update_type > UPDATE_TYPE_FAST) - WARN(1, "Global lock should NOT be set, overall_update_type should be UPDATE_TYPE_FAST"); - if (overall_update_type > UPDATE_TYPE_FAST) { ret = dm_atomic_get_state(state, &dm_state); -- 2.17.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 12/20] drm/amd/display: Rework DC plane filling and surface updates
From: Nicholas Kazlauskas [Why] We currently don't do DC validation for medium or full updates where the plane state isn't created. There are some medium and full updates that can cause bandwidth or clock changes to occur resulting in underflow or corruption. We need to be able to fill surface and plane info updates during atomic commit for dm_determine_update_type for commit. Since we already do this during atomic commit tail it would be good if we had the same logic in both places for creating these structures. [How] Introduce fill_dc_scaling_info and fill_dc_plane_info_and_addr. These two functions cover the following three update structures: - struct dc_scaling_info - struct dc_plane_info - struct dc_plane_address Cleanup and adapter the existing fill_plane_* helpers to work with these functions. Update call sites that used most of these sub helpers directly to work with the new functions. The exception being prepare_fb - we just want the new buffer attributes specifically in the case where we're creating the plane. This is needed for dc_commit_state in the case where the FB hasn't been previously been used. This isn't quite a refactor, but functionally driver behavior should be mostly the smae as before. The one exception is that we now check the return code for fill_plane_buffer_attributes which means that commits will be rejected that try to enable DCC with erroneous parameters. Change-Id: Ie9ced8c42f9c62afd83169300073f616f74f0724 Signed-off-by: Nicholas Kazlauskas Reviewed-by: David Francis Acked-by: Bhawanpreet Lakha --- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 406 +- 1 file changed, 204 insertions(+), 202 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 ee4738636aa0..2e5689412871 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -2458,59 +2458,48 @@ static const struct drm_encoder_funcs amdgpu_dm_encoder_funcs = { .destroy = amdgpu_dm_encoder_destroy, }; -static bool fill_rects_from_plane_state(const struct drm_plane_state *state, - struct rect *src_rect, - struct rect *dst_rect, - struct rect *clip_rect, - enum dc_rotation_angle *rotation) -{ - src_rect->x = state->src_x >> 16; - src_rect->y = state->src_y >> 16; - /* we ignore the mantissa for now and do not deal with floating pixels :( */ - src_rect->width = state->src_w >> 16; - - if (src_rect->width == 0) - return false; - src_rect->height = state->src_h >> 16; - if (src_rect->height == 0) - return false; +static int fill_dc_scaling_info(const struct drm_plane_state *state, + struct dc_scaling_info *scaling_info) +{ + memset(scaling_info, 0, sizeof(*scaling_info)); - dst_rect->x = state->crtc_x; - dst_rect->y = state->crtc_y; + /* Source is fixed 16.16 but we ignore mantissa for now... */ + scaling_info->src_rect.x = state->src_x >> 16; + scaling_info->src_rect.y = state->src_y >> 16; + + scaling_info->src_rect.width = state->src_w >> 16; + if (scaling_info->src_rect.width == 0) + return -EINVAL; + + scaling_info->src_rect.height = state->src_h >> 16; + if (scaling_info->src_rect.height == 0) + return -EINVAL; + + scaling_info->dst_rect.x = state->crtc_x; + scaling_info->dst_rect.y = state->crtc_y; if (state->crtc_w == 0) - return false; + return -EINVAL; - dst_rect->width = state->crtc_w; + scaling_info->dst_rect.width = state->crtc_w; if (state->crtc_h == 0) - return false; + return -EINVAL; - dst_rect->height = state->crtc_h; + scaling_info->dst_rect.height = state->crtc_h; - *clip_rect = *dst_rect; + /* DRM doesn't specify clipping on destination output. */ + scaling_info->clip_rect = scaling_info->dst_rect; - switch (state->rotation & DRM_MODE_ROTATE_MASK) { - case DRM_MODE_ROTATE_0: - *rotation = ROTATION_ANGLE_0; - break; - case DRM_MODE_ROTATE_90: - *rotation = ROTATION_ANGLE_90; - break; - case DRM_MODE_ROTATE_180: - *rotation = ROTATION_ANGLE_180; - break; - case DRM_MODE_ROTATE_270: - *rotation = ROTATION_ANGLE_270; - break; - default: - *rotation = ROTATION_ANGLE_0; - break; - } + /* +* The "scaling_quality" can be ignored for now, quality = 0 has DC +* assume reasonable defaults based on the format. +*/ - return true; + return 0;
[PATCH 20/20] drm/amd/display: Add profiling tools for bandwidth validation
From: Joshua Aberback [Why] We used this change to investigate the performance of bandwidth validation, it will be useful to have if we need to investigate further. [How] We use performance counter tick numbers to profile performance, they live at dc->debug.bw_val_profile (set .enable in debugger to turn on measuring). Change-Id: I828306882837c77c92a66f5c2b9d2de296685416 Signed-off-by: Joshua Aberback Reviewed-by: Tony Cheng Acked-by: Bhawanpreet Lakha --- .../gpu/drm/amd/display/dc/calcs/dcn_calcs.c | 18 +++ drivers/gpu/drm/amd/display/dc/dc.h | 52 +++ 2 files changed, 70 insertions(+) diff --git a/drivers/gpu/drm/amd/display/dc/calcs/dcn_calcs.c b/drivers/gpu/drm/amd/display/dc/calcs/dcn_calcs.c index d7aece82e4fe..1b4b51657f5e 100644 --- a/drivers/gpu/drm/amd/display/dc/calcs/dcn_calcs.c +++ b/drivers/gpu/drm/amd/display/dc/calcs/dcn_calcs.c @@ -704,6 +704,12 @@ bool dcn_validate_bandwidth( struct dc_state *context, bool fast_validate) { + /* +* we want a breakdown of the various stages of validation, which the +* perf_trace macro doesn't support +*/ + BW_VAL_TRACE_SETUP(); + const struct resource_pool *pool = dc->res_pool; struct dcn_bw_internal_vars *v = &context->dcn_bw_vars; int i, input_idx; @@ -712,6 +718,9 @@ bool dcn_validate_bandwidth( float bw_limit; PERFORMANCE_TRACE_START(); + + BW_VAL_TRACE_COUNT(); + if (dcn_bw_apply_registry_override(dc)) dcn_bw_sync_calcs_and_dml(dc); @@ -1014,6 +1023,8 @@ bool dcn_validate_bandwidth( mode_support_and_system_configuration(v); } + BW_VAL_TRACE_END_VOLTAGE_LEVEL(); + if (v->voltage_level != number_of_states_plus_one && !fast_validate) { float bw_consumed = v->total_bandwidth_consumed_gbyte_per_second; @@ -1089,6 +1100,8 @@ bool dcn_validate_bandwidth( break; } + BW_VAL_TRACE_END_WATERMARKS(); + for (i = 0, input_idx = 0; i < pool->pipe_count; i++) { struct pipe_ctx *pipe = &context->res_ctx.pipe_ctx[i]; @@ -1179,6 +1192,10 @@ bool dcn_validate_bandwidth( input_idx++; } + } else if (v->voltage_level == number_of_states_plus_one) { + BW_VAL_TRACE_SKIP(fail); + } else if (fast_validate) { + BW_VAL_TRACE_SKIP(fast); } if (v->voltage_level == 0) { @@ -1198,6 +1215,7 @@ bool dcn_validate_bandwidth( kernel_fpu_end(); PERFORMANCE_TRACE_END(); + BW_VAL_TRACE_FINISH(); if (bw_limit_pass && v->voltage_level != 5) return true; diff --git a/drivers/gpu/drm/amd/display/dc/dc.h b/drivers/gpu/drm/amd/display/dc/dc.h index 72bebd7523b8..55577e23f18e 100644 --- a/drivers/gpu/drm/amd/display/dc/dc.h +++ b/drivers/gpu/drm/amd/display/dc/dc.h @@ -249,6 +249,57 @@ struct dc_clocks { bool p_state_change_support; }; +struct dc_bw_validation_profile { + bool enable; + + unsigned long long total_ticks; + unsigned long long voltage_level_ticks; + unsigned long long watermark_ticks; + unsigned long long rq_dlg_ticks; + + unsigned long long total_count; + unsigned long long skip_fast_count; + unsigned long long skip_pass_count; + unsigned long long skip_fail_count; +}; + +#define BW_VAL_TRACE_SETUP() \ + unsigned long long end_tick = 0; \ + unsigned long long voltage_level_tick = 0; \ + unsigned long long watermark_tick = 0; \ + unsigned long long start_tick = dc->debug.bw_val_profile.enable ? \ + dm_get_timestamp(dc->ctx) : 0 + +#define BW_VAL_TRACE_COUNT() \ + if (dc->debug.bw_val_profile.enable) \ + dc->debug.bw_val_profile.total_count++ + +#define BW_VAL_TRACE_SKIP(status) \ + if (dc->debug.bw_val_profile.enable) { \ + if (!voltage_level_tick) \ + voltage_level_tick = dm_get_timestamp(dc->ctx); \ + dc->debug.bw_val_profile.skip_ ## status ## _count++; \ + } + +#define BW_VAL_TRACE_END_VOLTAGE_LEVEL() \ + if (dc->debug.bw_val_profile.enable) \ + voltage_level_tick = dm_get_timestamp(dc->ctx) + +#define BW_VAL_TRACE_END_WATERMARKS() \ + if (dc->debug.bw_val_profile.enable) \ + watermark_tick = dm_get_timestamp(dc->ctx) + +#define BW_VAL_TRACE_FINISH() \ + if (dc->debug.bw_val_profile.enable) { \ + end_tick = dm_get_timestamp(dc->ctx); \ + dc->debug.bw_val_profile.total_ticks += end_tick - start_tick; \ + dc->debug.bw_val_profile.voltage_level_ticks += voltage_le
[PATCH 17/20] drm/amd/display: Check scaling info when determing update type
From: Nicholas Kazlauskas [Why] Surface scaling info updates can affect bandwidth and blocks. We need to be checking these with global validation to avoid underflow or corruption. [How] Drop the state->allow_modeset early exit in dm_determine_update_type_for_commit. Most of those should be considered fast now anyway. Fill in scaling info and it to the surface update in atomic check. Change-Id: I3d32b8426a8097bb34851c51dfe143a75707f08d Signed-off-by: Nicholas Kazlauskas Reviewed-by: David Francis Acked-by: Bhawanpreet Lakha --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 11 --- 1 file changed, 8 insertions(+), 3 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 3aa1b9c05f88..3ff87bc31a8d 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -6406,6 +6406,7 @@ dm_determine_update_type_for_commit(struct amdgpu_display_manager *dm, for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) { struct dc_stream_update stream_update = { 0 }; + struct dc_scaling_info scaling_info; new_dm_crtc_state = to_dm_crtc_state(new_crtc_state); old_dm_crtc_state = to_dm_crtc_state(old_crtc_state); @@ -6433,9 +6434,6 @@ dm_determine_update_type_for_commit(struct amdgpu_display_manager *dm, goto cleanup; } - if (!state->allow_modeset) - continue; - if (crtc != new_plane_crtc) continue; @@ -6457,6 +6455,13 @@ dm_determine_update_type_for_commit(struct amdgpu_display_manager *dm, new_dm_crtc_state->stream->out_transfer_func; } + ret = fill_dc_scaling_info(new_plane_state, + &scaling_info); + if (ret) + goto cleanup; + + updates[num_plane].scaling_info = &scaling_info; + num_plane++; } -- 2.17.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 19/20] drm/amd/display: Add fast_validate parameter
From: Joshua Aberback Add a fast_validate parameter in dc_validate_global_state for future use Change-Id: If7a7ea618ba85bdddc8ee4419cd01e2fae3fda93 Signed-off-by: Joshua Aberback Reviewed-by: Tony Cheng Acked-by: Bhawanpreet Lakha --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +- drivers/gpu/drm/amd/display/dc/calcs/dcn_calcs.c| 6 -- drivers/gpu/drm/amd/display/dc/core/dc_resource.c | 6 -- drivers/gpu/drm/amd/display/dc/dc.h | 7 ++- drivers/gpu/drm/amd/display/dc/dce100/dce100_resource.c | 3 ++- drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c | 3 ++- drivers/gpu/drm/amd/display/dc/dce112/dce112_resource.c | 3 ++- drivers/gpu/drm/amd/display/dc/dce112/dce112_resource.h | 3 ++- drivers/gpu/drm/amd/display/dc/dce80/dce80_resource.c | 3 ++- drivers/gpu/drm/amd/display/dc/inc/core_types.h | 3 ++- drivers/gpu/drm/amd/display/dc/inc/dcn_calcs.h | 3 ++- 11 files changed, 29 insertions(+), 13 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 6aef4a3574db..bb7f6bc80830 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -6738,7 +6738,7 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev, if (ret) goto fail; - if (dc_validate_global_state(dc, dm_state->context) != DC_OK) { + if (dc_validate_global_state(dc, dm_state->context, false) != DC_OK) { ret = -EINVAL; goto fail; } diff --git a/drivers/gpu/drm/amd/display/dc/calcs/dcn_calcs.c b/drivers/gpu/drm/amd/display/dc/calcs/dcn_calcs.c index 8843361e842d..d7aece82e4fe 100644 --- a/drivers/gpu/drm/amd/display/dc/calcs/dcn_calcs.c +++ b/drivers/gpu/drm/amd/display/dc/calcs/dcn_calcs.c @@ -701,7 +701,8 @@ static void hack_bounding_box(struct dcn_bw_internal_vars *v, bool dcn_validate_bandwidth( struct dc *dc, - struct dc_state *context) + struct dc_state *context, + bool fast_validate) { const struct resource_pool *pool = dc->res_pool; struct dcn_bw_internal_vars *v = &context->dcn_bw_vars; @@ -1013,8 +1014,9 @@ bool dcn_validate_bandwidth( mode_support_and_system_configuration(v); } - if (v->voltage_level != 5) { + if (v->voltage_level != number_of_states_plus_one && !fast_validate) { float bw_consumed = v->total_bandwidth_consumed_gbyte_per_second; + if (bw_consumed < v->fabric_and_dram_bandwidth_vmin0p65) bw_consumed = v->fabric_and_dram_bandwidth_vmin0p65; else if (bw_consumed < v->fabric_and_dram_bandwidth_vmid0p72) diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c index 3830e6ce1355..eac7186e4f08 100644 --- a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c +++ b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c @@ -2067,12 +2067,14 @@ void dc_resource_state_construct( * Checks HW resource availability and bandwidth requirement. * @dc: dc struct for this driver * @new_ctx: state to be validated + * @fast_validate: set to true if only yes/no to support matters * * Return: DC_OK if the result can be programmed. Otherwise, an error code. */ enum dc_status dc_validate_global_state( struct dc *dc, - struct dc_state *new_ctx) + struct dc_state *new_ctx, + bool fast_validate) { enum dc_status result = DC_ERROR_UNEXPECTED; int i, j; @@ -2127,7 +2129,7 @@ enum dc_status dc_validate_global_state( result = resource_build_scaling_params_for_context(dc, new_ctx); if (result == DC_OK) - if (!dc->res_pool->funcs->validate_bandwidth(dc, new_ctx)) + if (!dc->res_pool->funcs->validate_bandwidth(dc, new_ctx, fast_validate)) result = DC_FAIL_BANDWIDTH_VALIDATE; return result; diff --git a/drivers/gpu/drm/amd/display/dc/dc.h b/drivers/gpu/drm/amd/display/dc/dc.h index 4222ffed11ff..72bebd7523b8 100644 --- a/drivers/gpu/drm/amd/display/dc/dc.h +++ b/drivers/gpu/drm/amd/display/dc/dc.h @@ -675,9 +675,14 @@ enum dc_status dc_validate_plane(struct dc *dc, const struct dc_plane_state *pla void get_clock_requirements_for_state(struct dc_state *state, struct AsicStateEx *info); +/* + * fast_validate: we return after determining if we can support the new state, + * but before we populate the programming info + */ enum dc_status dc_validate_global_state( struct dc *dc, - struct dc_state *new_ctx); + struct dc_state *new_ctx, + bool fast_validate); void dc_resource_state_construct( diff --git a/drivers/gpu/d
Re: [PATCH RFC tip/core/rcu 0/4] Forbid static SRCU use in modules
On Tue, Apr 09, 2019 at 11:56:03AM -0400, Mathieu Desnoyers wrote: > - On Apr 9, 2019, at 11:40 AM, Joel Fernandes, Google > j...@joelfernandes.org wrote: > > > On Mon, Apr 08, 2019 at 01:24:47PM -0400, Mathieu Desnoyers wrote: > >> - On Apr 8, 2019, at 11:46 AM, paulmck paul...@linux.ibm.com wrote: > >> > >> > On Mon, Apr 08, 2019 at 10:49:32AM -0400, Mathieu Desnoyers wrote: > >> >> - On Apr 8, 2019, at 10:22 AM, paulmck paul...@linux.ibm.com wrote: > >> >> > >> >> > On Mon, Apr 08, 2019 at 09:05:34AM -0400, Mathieu Desnoyers wrote: > >> >> >> - On Apr 7, 2019, at 10:27 PM, paulmck paul...@linux.ibm.com > >> >> >> wrote: > >> >> >> > >> >> >> > On Sun, Apr 07, 2019 at 09:07:18PM +, Joel Fernandes wrote: > >> >> >> >> On Sun, Apr 07, 2019 at 04:41:36PM -0400, Mathieu Desnoyers wrote: > >> >> >> >> > > >> >> >> >> > - On Apr 7, 2019, at 3:32 PM, Joel Fernandes, Google > >> >> >> >> > j...@joelfernandes.org > >> >> >> >> > wrote: > >> >> >> >> > > >> >> >> >> > > On Sun, Apr 07, 2019 at 03:26:16PM -0400, Mathieu Desnoyers > >> >> >> >> > > wrote: > >> >> >> >> > >> - On Apr 7, 2019, at 9:59 AM, paulmck > >> >> >> >> > >> paul...@linux.ibm.com wrote: > >> >> >> >> > >> > >> >> >> >> > >> > On Sun, Apr 07, 2019 at 06:39:41AM -0700, Paul E. McKenney > >> >> >> >> > >> > wrote: > >> >> >> >> > >> >> On Sat, Apr 06, 2019 at 07:06:13PM -0400, Joel Fernandes > >> >> >> >> > >> >> wrote: > >> >> >> >> > >> > > >> >> >> >> > >> > [ . . . ] > >> >> >> >> > >> > > >> >> >> >> > >> >> > > diff --git a/include/asm-generic/vmlinux.lds.h > >> >> >> >> > >> >> > > b/include/asm-generic/vmlinux.lds.h > >> >> >> >> > >> >> > > index f8f6f04c4453..c2d919a1566e 100644 > >> >> >> >> > >> >> > > --- a/include/asm-generic/vmlinux.lds.h > >> >> >> >> > >> >> > > +++ b/include/asm-generic/vmlinux.lds.h > >> >> >> >> > >> >> > > @@ -338,6 +338,10 @@ > >> >> >> >> > >> >> > > KEEP(*(__tracepoints_ptrs)) /* > >> >> >> >> > >> >> > > Tracepoints: pointer array */ \ > >> >> >> >> > >> >> > > __stop___tracepoints_ptrs = .; > >> >> >> >> > >> >> > > \ > >> >> >> >> > >> >> > > *(__tracepoints_strings)/* Tracepoints: > >> >> >> >> > >> >> > > strings */ \ > >> >> >> >> > >> >> > > + . = ALIGN(8); > >> >> >> >> > >> >> > > \ > >> >> >> >> > >> >> > > + __start___srcu_struct = .; > >> >> >> >> > >> >> > > \ > >> >> >> >> > >> >> > > + *(___srcu_struct_ptrs) > >> >> >> >> > >> >> > > \ > >> >> >> >> > >> >> > > + __end___srcu_struct = .; > >> >> >> >> > >> >> > > \ > >> >> >> >> > >> >> > > } > >> >> >> >> > >> >> > > \ > >> >> >> >> > >> >> > > >> >> >> >> > >> >> > This vmlinux linker modification is not needed. I > >> >> >> >> > >> >> > tested without it and srcu > >> >> >> >> > >> >> > torture works fine with rcutorture built as a module. > >> >> >> >> > >> >> > Putting further prints > >> >> >> >> > >> >> > in kernel/module.c verified that the kernel is able to > >> >> >> >> > >> >> > find the srcu structs > >> >> >> >> > >> >> > just fine. You could squash the below patch into this > >> >> >> >> > >> >> > one or apply it on top > >> >> >> >> > >> >> > of the dev branch. > >> >> >> >> > >> >> > >> >> >> >> > >> >> Good point, given that otherwise FORTRAN named common > >> >> >> >> > >> >> blocks would not > >> >> >> >> > >> >> work. > >> >> >> >> > >> >> > >> >> >> >> > >> >> But isn't one advantage of leaving that stuff in the > >> >> >> >> > >> >> RO_DATA_SECTION() > >> >> >> >> > >> >> macro that it can be mapped read-only? Or am I suffering > >> >> >> >> > >> >> from excessive > >> >> >> >> > >> >> optimism? > >> >> >> >> > >> > > >> >> >> >> > >> > And to answer the other question, in the case where I am > >> >> >> >> > >> > suffering from > >> >> >> >> > >> > excessive optimism, it should be a separate commit. > >> >> >> >> > >> > Please see below > >> >> >> >> > >> > for the updated original commit thus far. > >> >> >> >> > >> > > >> >> >> >> > >> > And may I have your Tested-by? > >> >> >> >> > >> > >> >> >> >> > >> Just to confirm: does the cleanup performed in the modules > >> >> >> >> > >> going > >> >> >> >> > >> notifier end up acting as a barrier first before freeing the > >> >> >> >> > >> memory ? > >> >> >> >> > >> If not, is it explicitly stated that a barrier must be > >> >> >> >> > >> issued before > >> >> >> >> > >> module unload ? > >> >> >> >> > >> > >> >> >> >> > > > >> >> >> >> > > You mean rcu_barrier? It is mentioned in the documentation > >> >> >> >> > > that this is the > >> >> >> >> > > responsibility of the module writer to prevent delays for all > >> >> >> >> > > modules. > >> >> >> >> > > >> >> >> >> > It's a srcu
Re: [PATCH RFC tip/core/rcu 0/4] Forbid static SRCU use in modules
- On Apr 9, 2019, at 1:55 PM, paulmck paul...@linux.ibm.com wrote: [...] > The current state is not horrible, so my thought would be to give it > some time to see if better thoughts arise. > > Either way, cleanup_srcu_struct() keeps its current checks for callbacks > still being in flight, which is why I believe that the current state is > not horrible. ;-) In that case, I think the comment above cleanup_srcu_struct_quiesced() in include/linux/srcu.h needs to be updated to cover situations where API users statically define a SRCU domain in a module and intend to unload that module. Given that we end up doing the allocation/cleanup under the hood, the API users don't interact with init_srcu_struct() nor cleanup_srcu_struct(), so it's not obvious that this comment also applies to them. Thanks, Mathieu -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH RFC tip/core/rcu 0/4] Forbid static SRCU use in modules
- On Apr 9, 2019, at 12:40 PM, paulmck paul...@linux.ibm.com wrote: > On Tue, Apr 09, 2019 at 11:56:03AM -0400, Mathieu Desnoyers wrote: >> - On Apr 9, 2019, at 11:40 AM, Joel Fernandes, Google >> j...@joelfernandes.org >> wrote: >> >> > On Mon, Apr 08, 2019 at 01:24:47PM -0400, Mathieu Desnoyers wrote: >> >> - On Apr 8, 2019, at 11:46 AM, paulmck paul...@linux.ibm.com wrote: >> >> >> >> > On Mon, Apr 08, 2019 at 10:49:32AM -0400, Mathieu Desnoyers wrote: >> >> >> - On Apr 8, 2019, at 10:22 AM, paulmck paul...@linux.ibm.com wrote: >> >> >> >> >> >> > On Mon, Apr 08, 2019 at 09:05:34AM -0400, Mathieu Desnoyers wrote: >> >> >> >> - On Apr 7, 2019, at 10:27 PM, paulmck paul...@linux.ibm.com >> >> >> >> wrote: >> >> >> >> >> >> >> >> > On Sun, Apr 07, 2019 at 09:07:18PM +, Joel Fernandes wrote: >> >> >> >> >> On Sun, Apr 07, 2019 at 04:41:36PM -0400, Mathieu Desnoyers >> >> >> >> >> wrote: >> >> >> >> >> > >> >> >> >> >> > - On Apr 7, 2019, at 3:32 PM, Joel Fernandes, Google >> >> >> >> >> > j...@joelfernandes.org >> >> >> >> >> > wrote: >> >> >> >> >> > >> >> >> >> >> > > On Sun, Apr 07, 2019 at 03:26:16PM -0400, Mathieu Desnoyers >> >> >> >> >> > > wrote: >> >> >> >> >> > >> - On Apr 7, 2019, at 9:59 AM, paulmck >> >> >> >> >> > >> paul...@linux.ibm.com wrote: >> >> >> >> >> > >> >> >> >> >> >> > >> > On Sun, Apr 07, 2019 at 06:39:41AM -0700, Paul E. >> >> >> >> >> > >> > McKenney wrote: >> >> >> >> >> > >> >> On Sat, Apr 06, 2019 at 07:06:13PM -0400, Joel Fernandes >> >> >> >> >> > >> >> wrote: >> >> >> >> >> > >> > >> >> >> >> >> > >> > [ . . . ] >> >> >> >> >> > >> > >> >> >> >> >> > >> >> > > diff --git a/include/asm-generic/vmlinux.lds.h >> >> >> >> >> > >> >> > > b/include/asm-generic/vmlinux.lds.h >> >> >> >> >> > >> >> > > index f8f6f04c4453..c2d919a1566e 100644 >> >> >> >> >> > >> >> > > --- a/include/asm-generic/vmlinux.lds.h >> >> >> >> >> > >> >> > > +++ b/include/asm-generic/vmlinux.lds.h >> >> >> >> >> > >> >> > > @@ -338,6 +338,10 @@ >> >> >> >> >> > >> >> > > KEEP(*(__tracepoints_ptrs)) /* >> >> >> >> >> > >> >> > > Tracepoints: pointer array */ \ >> >> >> >> >> > >> >> > > __stop___tracepoints_ptrs = .; >> >> >> >> >> > >> >> > > \ >> >> >> >> >> > >> >> > > *(__tracepoints_strings)/* Tracepoints: >> >> >> >> >> > >> >> > > strings */ \ >> >> >> >> >> > >> >> > > +. = ALIGN(8); >> >> >> >> >> > >> >> > > \ >> >> >> >> >> > >> >> > > +__start___srcu_struct = .; >> >> >> >> >> > >> >> > > \ >> >> >> >> >> > >> >> > > +*(___srcu_struct_ptrs) >> >> >> >> >> > >> >> > > \ >> >> >> >> >> > >> >> > > +__end___srcu_struct = .; >> >> >> >> >> > >> >> > > \ >> >> >> >> >> > >> >> > > } >> >> >> >> >> > >> >> > > \ >> >> >> >> >> > >> >> > >> >> >> >> >> > >> >> > This vmlinux linker modification is not needed. I >> >> >> >> >> > >> >> > tested without it and srcu >> >> >> >> >> > >> >> > torture works fine with rcutorture built as a module. >> >> >> >> >> > >> >> > Putting further prints >> >> >> >> >> > >> >> > in kernel/module.c verified that the kernel is able to >> >> >> >> >> > >> >> > find the srcu structs >> >> >> >> >> > >> >> > just fine. You could squash the below patch into this >> >> >> >> >> > >> >> > one or apply it on top >> >> >> >> >> > >> >> > of the dev branch. >> >> >> >> >> > >> >> >> >> >> >> >> > >> >> Good point, given that otherwise FORTRAN named common >> >> >> >> >> > >> >> blocks would not >> >> >> >> >> > >> >> work. >> >> >> >> >> > >> >> >> >> >> >> >> > >> >> But isn't one advantage of leaving that stuff in the >> >> >> >> >> > >> >> RO_DATA_SECTION() >> >> >> >> >> > >> >> macro that it can be mapped read-only? Or am I >> >> >> >> >> > >> >> suffering from excessive >> >> >> >> >> > >> >> optimism? >> >> >> >> >> > >> > >> >> >> >> >> > >> > And to answer the other question, in the case where I am >> >> >> >> >> > >> > suffering from >> >> >> >> >> > >> > excessive optimism, it should be a separate commit. >> >> >> >> >> > >> > Please see below >> >> >> >> >> > >> > for the updated original commit thus far. >> >> >> >> >> > >> > >> >> >> >> >> > >> > And may I have your Tested-by? >> >> >> >> >> > >> >> >> >> >> >> > >> Just to confirm: does the cleanup performed in the modules >> >> >> >> >> > >> going >> >> >> >> >> > >> notifier end up acting as a barrier first before freeing >> >> >> >> >> > >> the memory ? >> >> >> >> >> > >> If not, is it explicitly stated that a barrier must be >> >> >> >> >> > >> issued before >> >> >> >> >> > >> module unload ? >> >> >> >> >> > >> >> >> >> >> >> > > >> >> >> >> >> > > You mean rcu_barrier? It is mentioned in the documentation >> >> >>
Re: [PATCH RFC tip/core/rcu 0/4] Forbid static SRCU use in modules
On Tue, Apr 09, 2019 at 11:56:03AM -0400, Mathieu Desnoyers wrote: > - On Apr 9, 2019, at 11:40 AM, Joel Fernandes, Google > j...@joelfernandes.org wrote: > > > On Mon, Apr 08, 2019 at 01:24:47PM -0400, Mathieu Desnoyers wrote: > >> - On Apr 8, 2019, at 11:46 AM, paulmck paul...@linux.ibm.com wrote: > >> > >> > On Mon, Apr 08, 2019 at 10:49:32AM -0400, Mathieu Desnoyers wrote: > >> >> - On Apr 8, 2019, at 10:22 AM, paulmck paul...@linux.ibm.com wrote: > >> >> > >> >> > On Mon, Apr 08, 2019 at 09:05:34AM -0400, Mathieu Desnoyers wrote: > >> >> >> - On Apr 7, 2019, at 10:27 PM, paulmck paul...@linux.ibm.com > >> >> >> wrote: > >> >> >> > >> >> >> > On Sun, Apr 07, 2019 at 09:07:18PM +, Joel Fernandes wrote: > >> >> >> >> On Sun, Apr 07, 2019 at 04:41:36PM -0400, Mathieu Desnoyers wrote: > >> >> >> >> > > >> >> >> >> > - On Apr 7, 2019, at 3:32 PM, Joel Fernandes, Google > >> >> >> >> > j...@joelfernandes.org > >> >> >> >> > wrote: > >> >> >> >> > > >> >> >> >> > > On Sun, Apr 07, 2019 at 03:26:16PM -0400, Mathieu Desnoyers > >> >> >> >> > > wrote: > >> >> >> >> > >> - On Apr 7, 2019, at 9:59 AM, paulmck > >> >> >> >> > >> paul...@linux.ibm.com wrote: > >> >> >> >> > >> > >> >> >> >> > >> > On Sun, Apr 07, 2019 at 06:39:41AM -0700, Paul E. McKenney > >> >> >> >> > >> > wrote: > >> >> >> >> > >> >> On Sat, Apr 06, 2019 at 07:06:13PM -0400, Joel Fernandes > >> >> >> >> > >> >> wrote: > >> >> >> >> > >> > > >> >> >> >> > >> > [ . . . ] > >> >> >> >> > >> > > >> >> >> >> > >> >> > > diff --git a/include/asm-generic/vmlinux.lds.h > >> >> >> >> > >> >> > > b/include/asm-generic/vmlinux.lds.h > >> >> >> >> > >> >> > > index f8f6f04c4453..c2d919a1566e 100644 > >> >> >> >> > >> >> > > --- a/include/asm-generic/vmlinux.lds.h > >> >> >> >> > >> >> > > +++ b/include/asm-generic/vmlinux.lds.h > >> >> >> >> > >> >> > > @@ -338,6 +338,10 @@ > >> >> >> >> > >> >> > > KEEP(*(__tracepoints_ptrs)) /* > >> >> >> >> > >> >> > > Tracepoints: pointer array */ \ > >> >> >> >> > >> >> > > __stop___tracepoints_ptrs = .; > >> >> >> >> > >> >> > > \ > >> >> >> >> > >> >> > > *(__tracepoints_strings)/* Tracepoints: > >> >> >> >> > >> >> > > strings */ \ > >> >> >> >> > >> >> > > + . = ALIGN(8); > >> >> >> >> > >> >> > > \ > >> >> >> >> > >> >> > > + __start___srcu_struct = .; > >> >> >> >> > >> >> > > \ > >> >> >> >> > >> >> > > + *(___srcu_struct_ptrs) > >> >> >> >> > >> >> > > \ > >> >> >> >> > >> >> > > + __end___srcu_struct = .; > >> >> >> >> > >> >> > > \ > >> >> >> >> > >> >> > > } > >> >> >> >> > >> >> > > \ > >> >> >> >> > >> >> > > >> >> >> >> > >> >> > This vmlinux linker modification is not needed. I > >> >> >> >> > >> >> > tested without it and srcu > >> >> >> >> > >> >> > torture works fine with rcutorture built as a module. > >> >> >> >> > >> >> > Putting further prints > >> >> >> >> > >> >> > in kernel/module.c verified that the kernel is able to > >> >> >> >> > >> >> > find the srcu structs > >> >> >> >> > >> >> > just fine. You could squash the below patch into this > >> >> >> >> > >> >> > one or apply it on top > >> >> >> >> > >> >> > of the dev branch. > >> >> >> >> > >> >> > >> >> >> >> > >> >> Good point, given that otherwise FORTRAN named common > >> >> >> >> > >> >> blocks would not > >> >> >> >> > >> >> work. > >> >> >> >> > >> >> > >> >> >> >> > >> >> But isn't one advantage of leaving that stuff in the > >> >> >> >> > >> >> RO_DATA_SECTION() > >> >> >> >> > >> >> macro that it can be mapped read-only? Or am I suffering > >> >> >> >> > >> >> from excessive > >> >> >> >> > >> >> optimism? > >> >> >> >> > >> > > >> >> >> >> > >> > And to answer the other question, in the case where I am > >> >> >> >> > >> > suffering from > >> >> >> >> > >> > excessive optimism, it should be a separate commit. > >> >> >> >> > >> > Please see below > >> >> >> >> > >> > for the updated original commit thus far. > >> >> >> >> > >> > > >> >> >> >> > >> > And may I have your Tested-by? > >> >> >> >> > >> > >> >> >> >> > >> Just to confirm: does the cleanup performed in the modules > >> >> >> >> > >> going > >> >> >> >> > >> notifier end up acting as a barrier first before freeing the > >> >> >> >> > >> memory ? > >> >> >> >> > >> If not, is it explicitly stated that a barrier must be > >> >> >> >> > >> issued before > >> >> >> >> > >> module unload ? > >> >> >> >> > >> > >> >> >> >> > > > >> >> >> >> > > You mean rcu_barrier? It is mentioned in the documentation > >> >> >> >> > > that this is the > >> >> >> >> > > responsibility of the module writer to prevent delays for all > >> >> >> >> > > modules. > >> >> >> >> > > >> >> >> >> > It's a srcu
Re: [PATCH RFC tip/core/rcu 0/4] Forbid static SRCU use in modules
- On Apr 9, 2019, at 11:40 AM, Joel Fernandes, Google j...@joelfernandes.org wrote: > On Mon, Apr 08, 2019 at 01:24:47PM -0400, Mathieu Desnoyers wrote: >> - On Apr 8, 2019, at 11:46 AM, paulmck paul...@linux.ibm.com wrote: >> >> > On Mon, Apr 08, 2019 at 10:49:32AM -0400, Mathieu Desnoyers wrote: >> >> - On Apr 8, 2019, at 10:22 AM, paulmck paul...@linux.ibm.com wrote: >> >> >> >> > On Mon, Apr 08, 2019 at 09:05:34AM -0400, Mathieu Desnoyers wrote: >> >> >> - On Apr 7, 2019, at 10:27 PM, paulmck paul...@linux.ibm.com wrote: >> >> >> >> >> >> > On Sun, Apr 07, 2019 at 09:07:18PM +, Joel Fernandes wrote: >> >> >> >> On Sun, Apr 07, 2019 at 04:41:36PM -0400, Mathieu Desnoyers wrote: >> >> >> >> > >> >> >> >> > - On Apr 7, 2019, at 3:32 PM, Joel Fernandes, Google >> >> >> >> > j...@joelfernandes.org >> >> >> >> > wrote: >> >> >> >> > >> >> >> >> > > On Sun, Apr 07, 2019 at 03:26:16PM -0400, Mathieu Desnoyers >> >> >> >> > > wrote: >> >> >> >> > >> - On Apr 7, 2019, at 9:59 AM, paulmck >> >> >> >> > >> paul...@linux.ibm.com wrote: >> >> >> >> > >> >> >> >> >> > >> > On Sun, Apr 07, 2019 at 06:39:41AM -0700, Paul E. McKenney >> >> >> >> > >> > wrote: >> >> >> >> > >> >> On Sat, Apr 06, 2019 at 07:06:13PM -0400, Joel Fernandes >> >> >> >> > >> >> wrote: >> >> >> >> > >> > >> >> >> >> > >> > [ . . . ] >> >> >> >> > >> > >> >> >> >> > >> >> > > diff --git a/include/asm-generic/vmlinux.lds.h >> >> >> >> > >> >> > > b/include/asm-generic/vmlinux.lds.h >> >> >> >> > >> >> > > index f8f6f04c4453..c2d919a1566e 100644 >> >> >> >> > >> >> > > --- a/include/asm-generic/vmlinux.lds.h >> >> >> >> > >> >> > > +++ b/include/asm-generic/vmlinux.lds.h >> >> >> >> > >> >> > > @@ -338,6 +338,10 @@ >> >> >> >> > >> >> > > KEEP(*(__tracepoints_ptrs)) /* >> >> >> >> > >> >> > > Tracepoints: pointer array */ \ >> >> >> >> > >> >> > > __stop___tracepoints_ptrs = .; >> >> >> >> > >> >> > > \ >> >> >> >> > >> >> > > *(__tracepoints_strings)/* Tracepoints: >> >> >> >> > >> >> > > strings */ \ >> >> >> >> > >> >> > > + . = ALIGN(8); >> >> >> >> > >> >> > > \ >> >> >> >> > >> >> > > + __start___srcu_struct = .; >> >> >> >> > >> >> > > \ >> >> >> >> > >> >> > > + *(___srcu_struct_ptrs) >> >> >> >> > >> >> > > \ >> >> >> >> > >> >> > > + __end___srcu_struct = .; >> >> >> >> > >> >> > > \ >> >> >> >> > >> >> > > } >> >> >> >> > >> >> > > \ >> >> >> >> > >> >> > >> >> >> >> > >> >> > This vmlinux linker modification is not needed. I tested >> >> >> >> > >> >> > without it and srcu >> >> >> >> > >> >> > torture works fine with rcutorture built as a module. >> >> >> >> > >> >> > Putting further prints >> >> >> >> > >> >> > in kernel/module.c verified that the kernel is able to >> >> >> >> > >> >> > find the srcu structs >> >> >> >> > >> >> > just fine. You could squash the below patch into this one >> >> >> >> > >> >> > or apply it on top >> >> >> >> > >> >> > of the dev branch. >> >> >> >> > >> >> >> >> >> >> > >> >> Good point, given that otherwise FORTRAN named common >> >> >> >> > >> >> blocks would not >> >> >> >> > >> >> work. >> >> >> >> > >> >> >> >> >> >> > >> >> But isn't one advantage of leaving that stuff in the >> >> >> >> > >> >> RO_DATA_SECTION() >> >> >> >> > >> >> macro that it can be mapped read-only? Or am I suffering >> >> >> >> > >> >> from excessive >> >> >> >> > >> >> optimism? >> >> >> >> > >> > >> >> >> >> > >> > And to answer the other question, in the case where I am >> >> >> >> > >> > suffering from >> >> >> >> > >> > excessive optimism, it should be a separate commit. Please >> >> >> >> > >> > see below >> >> >> >> > >> > for the updated original commit thus far. >> >> >> >> > >> > >> >> >> >> > >> > And may I have your Tested-by? >> >> >> >> > >> >> >> >> >> > >> Just to confirm: does the cleanup performed in the modules >> >> >> >> > >> going >> >> >> >> > >> notifier end up acting as a barrier first before freeing the >> >> >> >> > >> memory ? >> >> >> >> > >> If not, is it explicitly stated that a barrier must be issued >> >> >> >> > >> before >> >> >> >> > >> module unload ? >> >> >> >> > >> >> >> >> >> > > >> >> >> >> > > You mean rcu_barrier? It is mentioned in the documentation that >> >> >> >> > > this is the >> >> >> >> > > responsibility of the module writer to prevent delays for all >> >> >> >> > > modules. >> >> >> >> > >> >> >> >> > It's a srcu barrier yes. Considering it would be a barrier >> >> >> >> > specific to the >> >> >> >> > srcu domain within that module, I don't see how it would cause >> >> >> >> > delays for >> >> >> >> > "all" modules if we implicitly issue the barrier on module >
Re: [PATCH RFC tip/core/rcu 0/4] Forbid static SRCU use in modules
On Tue, Apr 09, 2019 at 12:45:25PM -0400, Mathieu Desnoyers wrote: > - On Apr 9, 2019, at 12:40 PM, paulmck paul...@linux.ibm.com wrote: > > > On Tue, Apr 09, 2019 at 11:56:03AM -0400, Mathieu Desnoyers wrote: > >> - On Apr 9, 2019, at 11:40 AM, Joel Fernandes, Google > >> j...@joelfernandes.org > >> wrote: > >> > >> > On Mon, Apr 08, 2019 at 01:24:47PM -0400, Mathieu Desnoyers wrote: > >> >> - On Apr 8, 2019, at 11:46 AM, paulmck paul...@linux.ibm.com wrote: > >> >> > >> >> > On Mon, Apr 08, 2019 at 10:49:32AM -0400, Mathieu Desnoyers wrote: > >> >> >> - On Apr 8, 2019, at 10:22 AM, paulmck paul...@linux.ibm.com > >> >> >> wrote: > >> >> >> > >> >> >> > On Mon, Apr 08, 2019 at 09:05:34AM -0400, Mathieu Desnoyers wrote: > >> >> >> >> - On Apr 7, 2019, at 10:27 PM, paulmck paul...@linux.ibm.com > >> >> >> >> wrote: > >> >> >> >> > >> >> >> >> > On Sun, Apr 07, 2019 at 09:07:18PM +, Joel Fernandes wrote: > >> >> >> >> >> On Sun, Apr 07, 2019 at 04:41:36PM -0400, Mathieu Desnoyers > >> >> >> >> >> wrote: > >> >> >> >> >> > > >> >> >> >> >> > - On Apr 7, 2019, at 3:32 PM, Joel Fernandes, Google > >> >> >> >> >> > j...@joelfernandes.org > >> >> >> >> >> > wrote: > >> >> >> >> >> > > >> >> >> >> >> > > On Sun, Apr 07, 2019 at 03:26:16PM -0400, Mathieu > >> >> >> >> >> > > Desnoyers wrote: > >> >> >> >> >> > >> - On Apr 7, 2019, at 9:59 AM, paulmck > >> >> >> >> >> > >> paul...@linux.ibm.com wrote: > >> >> >> >> >> > >> > >> >> >> >> >> > >> > On Sun, Apr 07, 2019 at 06:39:41AM -0700, Paul E. > >> >> >> >> >> > >> > McKenney wrote: > >> >> >> >> >> > >> >> On Sat, Apr 06, 2019 at 07:06:13PM -0400, Joel > >> >> >> >> >> > >> >> Fernandes wrote: > >> >> >> >> >> > >> > > >> >> >> >> >> > >> > [ . . . ] > >> >> >> >> >> > >> > > >> >> >> >> >> > >> >> > > diff --git a/include/asm-generic/vmlinux.lds.h > >> >> >> >> >> > >> >> > > b/include/asm-generic/vmlinux.lds.h > >> >> >> >> >> > >> >> > > index f8f6f04c4453..c2d919a1566e 100644 > >> >> >> >> >> > >> >> > > --- a/include/asm-generic/vmlinux.lds.h > >> >> >> >> >> > >> >> > > +++ b/include/asm-generic/vmlinux.lds.h > >> >> >> >> >> > >> >> > > @@ -338,6 +338,10 @@ > >> >> >> >> >> > >> >> > >KEEP(*(__tracepoints_ptrs)) /* > >> >> >> >> >> > >> >> > > Tracepoints: pointer array */ \ > >> >> >> >> >> > >> >> > >__stop___tracepoints_ptrs = .; > >> >> >> >> >> > >> >> > > \ > >> >> >> >> >> > >> >> > >*(__tracepoints_strings)/* Tracepoints: > >> >> >> >> >> > >> >> > > strings */ \ > >> >> >> >> >> > >> >> > > + . = ALIGN(8); > >> >> >> >> >> > >> >> > > \ > >> >> >> >> >> > >> >> > > + __start___srcu_struct = .; > >> >> >> >> >> > >> >> > > \ > >> >> >> >> >> > >> >> > > + *(___srcu_struct_ptrs) > >> >> >> >> >> > >> >> > > \ > >> >> >> >> >> > >> >> > > + __end___srcu_struct = .; > >> >> >> >> >> > >> >> > > \ > >> >> >> >> >> > >> >> > >} > >> >> >> >> >> > >> >> > > \ > >> >> >> >> >> > >> >> > > >> >> >> >> >> > >> >> > This vmlinux linker modification is not needed. I > >> >> >> >> >> > >> >> > tested without it and srcu > >> >> >> >> >> > >> >> > torture works fine with rcutorture built as a > >> >> >> >> >> > >> >> > module. Putting further prints > >> >> >> >> >> > >> >> > in kernel/module.c verified that the kernel is able > >> >> >> >> >> > >> >> > to find the srcu structs > >> >> >> >> >> > >> >> > just fine. You could squash the below patch into > >> >> >> >> >> > >> >> > this one or apply it on top > >> >> >> >> >> > >> >> > of the dev branch. > >> >> >> >> >> > >> >> > >> >> >> >> >> > >> >> Good point, given that otherwise FORTRAN named common > >> >> >> >> >> > >> >> blocks would not > >> >> >> >> >> > >> >> work. > >> >> >> >> >> > >> >> > >> >> >> >> >> > >> >> But isn't one advantage of leaving that stuff in the > >> >> >> >> >> > >> >> RO_DATA_SECTION() > >> >> >> >> >> > >> >> macro that it can be mapped read-only? Or am I > >> >> >> >> >> > >> >> suffering from excessive > >> >> >> >> >> > >> >> optimism? > >> >> >> >> >> > >> > > >> >> >> >> >> > >> > And to answer the other question, in the case where I > >> >> >> >> >> > >> > am suffering from > >> >> >> >> >> > >> > excessive optimism, it should be a separate commit. > >> >> >> >> >> > >> > Please see below > >> >> >> >> >> > >> > for the updated original commit thus far. > >> >> >> >> >> > >> > > >> >> >> >> >> > >> > And may I have your Tested-by? > >> >> >> >> >> > >> > >> >> >> >> >> > >> Just to confirm: does the cleanup performed in the > >> >> >> >> >> > >> modules going > >> >> >> >> >> > >> notifier end up acting as a barrier first before freeing > >> >> >> >> >> > >> the memory ? > >> >> >> >> >> > >> If no
Re: [PATCH RFC tip/core/rcu 0/4] Forbid static SRCU use in modules
On Tue, Apr 09, 2019 at 02:04:11PM -0400, Mathieu Desnoyers wrote: > - On Apr 9, 2019, at 1:55 PM, paulmck paul...@linux.ibm.com wrote: > [...] > > The current state is not horrible, so my thought would be to give it > > some time to see if better thoughts arise. > > > > Either way, cleanup_srcu_struct() keeps its current checks for callbacks > > still being in flight, which is why I believe that the current state is > > not horrible. ;-) > > In that case, I think the comment above cleanup_srcu_struct_quiesced() in > include/linux/srcu.h needs to be updated to cover situations where API > users statically define a SRCU domain in a module and intend to unload > that module. > > Given that we end up doing the allocation/cleanup under the hood, the > API users don't interact with init_srcu_struct() nor cleanup_srcu_struct(), > so it's not obvious that this comment also applies to them. Actually, it turned out that cleanup_srcu_struct_quiesced() is extremely hard to use correctly, and maybe even impossible to use correctly. So cleanup_srcu_struct_quiesced has been eliminated in current -rcu. Thanx, Paul > Thanks, > > Mathieu > > -- > Mathieu Desnoyers > EfficiOS Inc. > http://www.efficios.com > ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
RE: [PATCH 1/2] drm/powerplay: Add smu set xgmi pstate interface
Reviewed-by: Evan Quan However, if this is for vega20, please be noticed that vega20 still takes old powerplay routines at default. Regards, Evan > -Original Message- > From: amd-gfx On Behalf Of Liu, > Shaoyun > Sent: Saturday, April 06, 2019 12:02 AM > To: amd-gfx@lists.freedesktop.org > Cc: Liu, Shaoyun > Subject: [PATCH 1/2] drm/powerplay: Add smu set xgmi pstate interface > > XGMI pstate is controlled by SMU, driver need this interface to communicate > with SMU > > Change-Id: I3a30797332557725d48d392bea0c9d59e2d0e427 > Signed-off-by: shaoyunl > --- > drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h | 4 > drivers/gpu/drm/amd/powerplay/smu_v11_0.c | 8 > 2 files changed, 12 insertions(+) > > diff --git a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h > b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h > index 2083139..0e52d1b 100644 > --- a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h > +++ b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h > @@ -546,6 +546,8 @@ struct smu_funcs > int (*get_fan_speed_percent)(struct smu_context *smu, uint32_t > *speed); > int (*set_fan_speed_percent)(struct smu_context *smu, uint32_t > speed); > int (*set_fan_speed_rpm)(struct smu_context *smu, uint32_t > speed); > + int (*set_xgmi_pstate)(struct smu_context *smu, uint32_t pstate); > + > }; > > #define smu_init_microcode(smu) \ > @@ -729,6 +731,8 @@ struct smu_funcs > ((smu)->funcs->get_sclk ? (smu)->funcs->get_sclk((smu), (low)) : 0) > #define smu_get_mclk(smu, low) \ > ((smu)->funcs->get_mclk ? (smu)->funcs->get_mclk((smu), (low)) : 0) > +#define smu_set_xgmi_pstate(smu, pstate) \ > + ((smu)->funcs->set_xgmi_pstate ? (smu)->funcs- > >set_xgmi_pstate((smu), > +(pstate)) : 0) > > > extern int smu_get_atom_data_table(struct smu_context *smu, uint32_t > table, diff --git a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c > b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c > index 9d75911..2e2ce71 100644 > --- a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c > +++ b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c > @@ -1952,6 +1952,13 @@ static int smu_v11_0_set_fan_speed_rpm(struct > smu_context *smu, > return ret; > } > > +static int smu_v11_0_set_xgmi_pstate(struct smu_context *smu, > + uint32_t pstate) > +{ > + /* send msg to SMU to set pstate */ > + return 0; > +} > + > static const struct smu_funcs smu_v11_0_funcs = { > .init_microcode = smu_v11_0_init_microcode, > .load_microcode = smu_v11_0_load_microcode, @@ -2008,6 +2015,7 > @@ static const struct smu_funcs smu_v11_0_funcs = { > .get_fan_speed_percent = smu_v11_0_get_fan_speed_percent, > .set_fan_speed_percent = smu_v11_0_set_fan_speed_percent, > .set_fan_speed_rpm = smu_v11_0_set_fan_speed_rpm, > + .set_xgmi_pstate = smu_v11_0_set_xgmi_pstate, > }; > > void smu_v11_0_set_smu_funcs(struct smu_context *smu) > -- > 2.7.4 > > ___ > amd-gfx mailing list > amd-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
RE: [PATCH] drm/amdgpu: Always enable memory sharing within same XGMI hive
Acked-by: Evan Quan > -Original Message- > From: amd-gfx On Behalf Of Liu, > Shaoyun > Sent: Tuesday, April 09, 2019 10:55 PM > To: amd-gfx@lists.freedesktop.org > Subject: Re: [PATCH] drm/amdgpu: Always enable memory sharing within > same XGMI hive > > ping > > On 2019-04-08 6:32 p.m., Liu, Shaoyun wrote: > > XGMI Memory sharing will be disbaled by default for security reason > > after boot up, it depends on driver to enable the memory sharing > > > > Change-Id: Ib516066eecfb877f84f1460a4d659abea44adb02 > > Signed-off-by: shaoyunl > > --- > > drivers/gpu/drm/amd/amdgpu/psp_v11_0.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c > > b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c > > index 2b3429d..b91df7b 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c > > +++ b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c > > @@ -674,7 +674,7 @@ static int psp_v11_0_xgmi_set_topology_info(struct > psp_context *psp, > > for (i = 0; i < topology_info_input->num_nodes; i++) { > > topology_info_input->nodes[i].node_id = topology- > >nodes[i].node_id; > > topology_info_input->nodes[i].num_hops = topology- > >nodes[i].num_hops; > > - topology_info_input->nodes[i].is_sharing_enabled = > topology->nodes[i].is_sharing_enabled; > > + topology_info_input->nodes[i].is_sharing_enabled = 1; > > topology_info_input->nodes[i].sdma_engine = topology- > >nodes[i].sdma_engine; > > } > > > ___ > amd-gfx mailing list > amd-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
RE: [PATCH 2/2] drm/amdgpu: Set proper function to set xgmi pstate
'pstate' here (int) is with different data type from the 1st patch of this series(uint32_t there). Please check whether that can cause any potential issues. With that confirmed, the patch is reviewed-by: Evan Quan > -Original Message- > From: amd-gfx On Behalf Of Liu, > Shaoyun > Sent: Saturday, April 06, 2019 12:02 AM > To: amd-gfx@lists.freedesktop.org > Cc: Liu, Shaoyun > Subject: [PATCH 2/2] drm/amdgpu: Set proper function to set xgmi pstate > > Driver need to call SMU to set xgmi pstate > > Change-Id: Iad7fd0e3b3155e45be8fe9119686c5bafa3c176c > Signed-off-by: shaoyunl > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c | 13 - > 1 file changed, 12 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c > index 3368347..a48c84c 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c > @@ -24,6 +24,7 @@ > #include > #include "amdgpu.h" > #include "amdgpu_xgmi.h" > +#include "amdgpu_smu.h" > > > static DEFINE_MUTEX(xgmi_mutex); > @@ -216,7 +217,17 @@ int amdgpu_xgmi_set_pstate(struct amdgpu_device > *adev, int pstate) > > if (hive->pstate == pstate) > return 0; > - /* Todo : sent the message to SMU for pstate change */ > + > + dev_dbg(adev->dev, "Set xgmi pstate %d.\n", pstate); > + > + if (is_support_sw_smu(adev)) > + ret = smu_set_xgmi_pstate(&adev->smu, pstate); > + if (ret) > + dev_err(adev->dev, > + "XGMI: Set pstate failure on device %llx, hive %llx, > ret %d", > + adev->gmc.xgmi.node_id, > + adev->gmc.xgmi.hive_id, ret); > + > return ret; > } > > -- > 2.7.4 > > ___ > amd-gfx mailing list > amd-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx