[PATCH v2] drm: register more drm device nodes
Since the default number 256 can't handle large modern systems with large numbers of GPUs, specify a more reasonable default. -v2: update drm_core_exit to unregister more drm device nodes Signed-off-by: James Zhu --- drivers/gpu/drm/drm_drv.c | 4 ++-- include/drm/drm_drv.h | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index 243cacb3575c..bfc6c18a7a92 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -1063,7 +1063,7 @@ static void drm_core_exit(void) { drm_privacy_screen_lookup_exit(); accel_core_exit(); - unregister_chrdev(DRM_MAJOR, "drm"); + __unregister_chrdev(DRM_MAJOR, 0, DRM_CHRDEV_MAX, "drm"); debugfs_remove(drm_debugfs_root); drm_sysfs_destroy(); idr_destroy(&drm_minors_idr); @@ -1086,7 +1086,7 @@ static int __init drm_core_init(void) drm_debugfs_root = debugfs_create_dir("dri", NULL); - ret = register_chrdev(DRM_MAJOR, "drm", &drm_stub_fops); + ret = __register_chrdev(DRM_MAJOR, 0, DRM_CHRDEV_MAX, "drm", &drm_stub_fops); if (ret < 0) goto error; diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h index 8878260d7529..8a2da92f02b7 100644 --- a/include/drm/drm_drv.h +++ b/include/drm/drm_drv.h @@ -45,6 +45,8 @@ struct drm_mode_create_dumb; struct drm_printer; struct sg_table; +#define DRM_CHRDEV_MAX1024 + /** * enum drm_driver_feature - feature flags * -- 2.34.1
Re: [PATCH] drm: register more drm device nodes
Please ignore this. need update unregister_chrdev in the meantime Will send out v2 later, On 2024-08-29 15:51, James Zhu wrote: Ping ... On 2024-05-30 11:34, James Zhu wrote: Since the default number 256 can't handle large modern systems with large numbers of GPUs, specify a more reasonable default. Signed-off-by: James Zhu --- drivers/gpu/drm/drm_drv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index 243cacb3575c..719ea57a70ab 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -1086,7 +1086,7 @@ static int __init drm_core_init(void) drm_debugfs_root = debugfs_create_dir("dri", NULL); - ret = register_chrdev(DRM_MAJOR, "drm", &drm_stub_fops); + ret = __register_chrdev(DRM_MAJOR, 0, 1024, "drm", &drm_stub_fops); if (ret < 0) goto error;
Re: [PATCH] drm: register more drm device nodes
Ping ... On 2024-05-30 11:34, James Zhu wrote: Since the default number 256 can't handle large modern systems with large numbers of GPUs, specify a more reasonable default. Signed-off-by: James Zhu --- drivers/gpu/drm/drm_drv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index 243cacb3575c..719ea57a70ab 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -1086,7 +1086,7 @@ static int __init drm_core_init(void) drm_debugfs_root = debugfs_create_dir("dri", NULL); - ret = register_chrdev(DRM_MAJOR, "drm", &drm_stub_fops); + ret = __register_chrdev(DRM_MAJOR, 0, 1024, "drm", &drm_stub_fops); if (ret < 0) goto error;
Re: [PATCH v6 0/4] drm: Use full allocated minor range for DRM
Hi Michal I did give Tested-by before. If you need Acks, Here I can give Acked-by: James Zhu for the series Best Regards! James On 2024-08-13 20:18, Michał Winiarski wrote: On Mon, Aug 12, 2024 at 01:38:38PM GMT, Alex Deucher wrote: Are there any objections to this series? We have been running into this limit as a problem for a while now on big servers. I don't think there were any objections, just a general lack of interest - so there are no R-b / Acks. If you're interested to have a go at it - I can resend it. It should still apply on latest drm-tip. -Michał Alex On Mon, Jul 24, 2023 at 5:15 PM Michał Winiarski wrote: 64 DRM device nodes is not enough for everyone. Upgrade it to ~512K (which definitely is more than enough). To allow testing userspace support for >64 devices, add additional DRM modparam (force_extended_minors) which causes DRM to skip allocating minors in 0-192 range. Additionally - convert minors to use XArray instead of IDR to simplify the locking. v1 -> v2: Don't touch DRM_MINOR_CONTROL and its range (Simon Ser) v2 -> v3: Don't use legacy scheme for >=192 minor range (Dave Airlie) Add modparam for testing (Dave Airlie) Add lockdep annotation for IDR (Daniel Vetter) v3 -> v4: Convert from IDR to XArray (Matthew Wilcox) v4 -> v5: Fixup IDR to XArray conversion (Matthew Wilcox) v5 -> v6: Also convert Accel to XArray Rename skip_legacy_minors to force_extended_minors Michał Winiarski (4): drm: Use XArray instead of IDR for minors accel: Use XArray instead of IDR for minors drm: Expand max DRM device number to full MINORBITS drm: Introduce force_extended_minors modparam drivers/accel/drm_accel.c | 110 +++-- drivers/gpu/drm/drm_drv.c | 105 --- drivers/gpu/drm/drm_file.c | 2 +- drivers/gpu/drm/drm_internal.h | 4 -- include/drm/drm_accel.h| 18 +- include/drm/drm_file.h | 5 ++ 6 files changed, 69 insertions(+), 175 deletions(-) -- 2.41.0
Re: [PATCH v6 0/4] drm: Use full allocated minor range for DRM
Hi Michal I did give Tested-by before. If you need Acks, Here I can give Acked-by:JamesZhufortheseries Best Regards! James On 2024-08-13 20:18, Michał Winiarski wrote: On Mon, Aug 12, 2024 at 01:38:38PM GMT, Alex Deucher wrote: Are there any objections to this series? We have been running into this limit as a problem for a while now on big servers. I don't think there were any objections, just a general lack of interest - so there are no R-b / Acks. If you're interested to have a go at it - I can resend it. It should still apply on latest drm-tip. -Michał Alex On Mon, Jul 24, 2023 at 5:15 PM Michał Winiarski wrote: 64 DRM device nodes is not enough for everyone. Upgrade it to ~512K (which definitely is more than enough). To allow testing userspace support for >64 devices, add additional DRM modparam (force_extended_minors) which causes DRM to skip allocating minors in 0-192 range. Additionally - convert minors to use XArray instead of IDR to simplify the locking. v1 -> v2: Don't touch DRM_MINOR_CONTROL and its range (Simon Ser) v2 -> v3: Don't use legacy scheme for >=192 minor range (Dave Airlie) Add modparam for testing (Dave Airlie) Add lockdep annotation for IDR (Daniel Vetter) v3 -> v4: Convert from IDR to XArray (Matthew Wilcox) v4 -> v5: Fixup IDR to XArray conversion (Matthew Wilcox) v5 -> v6: Also convert Accel to XArray Rename skip_legacy_minors to force_extended_minors Michał Winiarski (4): drm: Use XArray instead of IDR for minors accel: Use XArray instead of IDR for minors drm: Expand max DRM device number to full MINORBITS drm: Introduce force_extended_minors modparam drivers/accel/drm_accel.c | 110 +++-- drivers/gpu/drm/drm_drv.c | 105 --- drivers/gpu/drm/drm_file.c | 2 +- drivers/gpu/drm/drm_internal.h | 4 -- include/drm/drm_accel.h| 18 +- include/drm/drm_file.h | 5 ++ 6 files changed, 69 insertions(+), 175 deletions(-) -- 2.41.0
[PATCH] drm: register more drm device nodes
Since the default number 256 can't handle large modern systems with large numbers of GPUs, specify a more reasonable default. Signed-off-by: James Zhu --- drivers/gpu/drm/drm_drv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index 243cacb3575c..719ea57a70ab 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -1086,7 +1086,7 @@ static int __init drm_core_init(void) drm_debugfs_root = debugfs_create_dir("dri", NULL); - ret = register_chrdev(DRM_MAJOR, "drm", &drm_stub_fops); + ret = __register_chrdev(DRM_MAJOR, 0, 1024, "drm", &drm_stub_fops); if (ret < 0) goto error; -- 2.34.1
Re: [PATCH v6 0/4] drm: Use full allocated minor range for DRM
PATCH 1 and 3 are Tested-by:JamesZhu Best Regards! James Zhu On 2023-07-24 17:14, Michał Winiarski wrote: 64 DRM device nodes is not enough for everyone. Upgrade it to ~512K (which definitely is more than enough). To allow testing userspace support for >64 devices, add additional DRM modparam (force_extended_minors) which causes DRM to skip allocating minors in 0-192 range. Additionally - convert minors to use XArray instead of IDR to simplify the locking. v1 -> v2: Don't touch DRM_MINOR_CONTROL and its range (Simon Ser) v2 -> v3: Don't use legacy scheme for >=192 minor range (Dave Airlie) Add modparam for testing (Dave Airlie) Add lockdep annotation for IDR (Daniel Vetter) v3 -> v4: Convert from IDR to XArray (Matthew Wilcox) v4 -> v5: Fixup IDR to XArray conversion (Matthew Wilcox) v5 -> v6: Also convert Accel to XArray Rename skip_legacy_minors to force_extended_minors Michał Winiarski (4): drm: Use XArray instead of IDR for minors accel: Use XArray instead of IDR for minors drm: Expand max DRM device number to full MINORBITS drm: Introduce force_extended_minors modparam drivers/accel/drm_accel.c | 110 +++-- drivers/gpu/drm/drm_drv.c | 105 --- drivers/gpu/drm/drm_file.c | 2 +- drivers/gpu/drm/drm_internal.h | 4 -- include/drm/drm_accel.h| 18 +- include/drm/drm_file.h | 5 ++ 6 files changed, 69 insertions(+), 175 deletions(-)
Re: [PATCH v6 1/4] drm: Use XArray instead of IDR for minors
On 2023-08-29 14:33, Matthew Wilcox wrote: On Tue, Aug 29, 2023 at 01:34:22PM -0400, James Zhu wrote: @@ -1067,7 +1055,7 @@ static void drm_core_exit(void) unregister_chrdev(DRM_MAJOR, "drm"); debugfs_remove(drm_debugfs_root); drm_sysfs_destroy(); - idr_destroy(&drm_minors_idr); [JZ] Should we call xa_destroy instead here? We could, if we expect the xarray to potentially not be empty. From what I understand - all minors should be released at this point. [JZ] In practice, adding destroy here will be better. Why do you say that? [JZ] In case, the future, INIT adds something, need DESTROY to free completely.
Re: [PATCH v6 1/4] drm: Use XArray instead of IDR for minors
On 2023-08-28 17:08, Michał Winiarski wrote: On Fri, Aug 25, 2023 at 12:59:26PM -0400, James Zhu wrote: On 2023-07-24 17:14, Michał Winiarski wrote: IDR is deprecated, and since XArray manages its own state with internal locking, it simplifies the locking on DRM side. Additionally, don't use the IRQ-safe variant, since operating on drm minor is not done in IRQ context. Signed-off-by: Michał Winiarski Suggested-by: Matthew Wilcox --- drivers/gpu/drm/drm_drv.c | 63 --- 1 file changed, 25 insertions(+), 38 deletions(-) diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index 3eda026ffac6..3faecb01186f 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -34,6 +34,7 @@ #include #include #include +#include #include #include @@ -54,8 +55,7 @@ MODULE_AUTHOR("Gareth Hughes, Leif Delgass, José Fonseca, Jon Smirl"); MODULE_DESCRIPTION("DRM shared core routines"); MODULE_LICENSE("GPL and additional rights"); -static DEFINE_SPINLOCK(drm_minor_lock); -static struct idr drm_minors_idr; +static DEFINE_XARRAY_ALLOC(drm_minors_xa); /* * If the drm core fails to init for whatever reason, @@ -101,26 +101,23 @@ static struct drm_minor **drm_minor_get_slot(struct drm_device *dev, static void drm_minor_alloc_release(struct drm_device *dev, void *data) { struct drm_minor *minor = data; - unsigned long flags; WARN_ON(dev != minor->dev); put_device(minor->kdev); - if (minor->type == DRM_MINOR_ACCEL) { + if (minor->type == DRM_MINOR_ACCEL) accel_minor_remove(minor->index); - } else { - spin_lock_irqsave(&drm_minor_lock, flags); - idr_remove(&drm_minors_idr, minor->index); - spin_unlock_irqrestore(&drm_minor_lock, flags); - } + else + xa_erase(&drm_minors_xa, minor->index); } +#define DRM_MINOR_LIMIT(t) ({ typeof(t) _t = (t); XA_LIMIT(64 * _t, 64 * _t + 63); }) + static int drm_minor_alloc(struct drm_device *dev, enum drm_minor_type type) { struct drm_minor *minor; - unsigned long flags; - int r; + int index, r; minor = drmm_kzalloc(dev, sizeof(*minor), GFP_KERNEL); if (!minor) @@ -129,24 +126,17 @@ static int drm_minor_alloc(struct drm_device *dev, enum drm_minor_type type) minor->type = type; minor->dev = dev; - idr_preload(GFP_KERNEL); if (type == DRM_MINOR_ACCEL) { r = accel_minor_alloc(); + index = r; } else { - spin_lock_irqsave(&drm_minor_lock, flags); - r = idr_alloc(&drm_minors_idr, - NULL, - 64 * type, - 64 * (type + 1), - GFP_NOWAIT); - spin_unlock_irqrestore(&drm_minor_lock, flags); + r = xa_alloc(&drm_minors_xa, &index, NULL, DRM_MINOR_LIMIT(type), GFP_KERNEL); } - idr_preload_end(); if (r < 0) return r; - minor->index = r; + minor->index = index; r = drmm_add_action_or_reset(dev, drm_minor_alloc_release, minor); if (r) @@ -163,7 +153,7 @@ static int drm_minor_alloc(struct drm_device *dev, enum drm_minor_type type) static int drm_minor_register(struct drm_device *dev, enum drm_minor_type type) { struct drm_minor *minor; - unsigned long flags; + void *entry; int ret; DRM_DEBUG("\n"); @@ -190,9 +180,12 @@ static int drm_minor_register(struct drm_device *dev, enum drm_minor_type type) if (minor->type == DRM_MINOR_ACCEL) { accel_minor_replace(minor, minor->index); } else { - spin_lock_irqsave(&drm_minor_lock, flags); - idr_replace(&drm_minors_idr, minor, minor->index); - spin_unlock_irqrestore(&drm_minor_lock, flags); + entry = xa_store(&drm_minors_xa, minor->index, minor, GFP_KERNEL); + if (xa_is_err(entry)) { + ret = xa_err(entry); + goto err_debugfs; + } + WARN_ON(entry); [JZ] would WARN_ON(entry != minor)be better? We expect NULL here. The same one that was previously stored here ⌄⌄⌄ r = xa_alloc(&drm_minors_xa, &minor->index, NULL, DRM_EXTENDED_MINOR_LIMIT, GFP_KERNEL); in drm_minor_alloc. [JZ] My mistake. } DRM_DEBUG("new minor registered %d\n", minor->index); @@ -206,20 +199,16 @@ static int drm_minor_register(struct drm_device *dev, enum drm_minor_type type) static void drm_minor_unregister(struct drm_device *dev, enum drm_minor_type type) { struct drm_minor *minor; - unsigned long flags; minor = *
Re: [PATCH v6 1/4] drm: Use XArray instead of IDR for minors
On 2023-07-24 17:14, Michał Winiarski wrote: IDR is deprecated, and since XArray manages its own state with internal locking, it simplifies the locking on DRM side. Additionally, don't use the IRQ-safe variant, since operating on drm minor is not done in IRQ context. Signed-off-by: Michał Winiarski Suggested-by: Matthew Wilcox --- drivers/gpu/drm/drm_drv.c | 63 --- 1 file changed, 25 insertions(+), 38 deletions(-) diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index 3eda026ffac6..3faecb01186f 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -34,6 +34,7 @@ #include #include #include +#include #include #include @@ -54,8 +55,7 @@ MODULE_AUTHOR("Gareth Hughes, Leif Delgass, José Fonseca, Jon Smirl"); MODULE_DESCRIPTION("DRM shared core routines"); MODULE_LICENSE("GPL and additional rights"); -static DEFINE_SPINLOCK(drm_minor_lock); -static struct idr drm_minors_idr; +static DEFINE_XARRAY_ALLOC(drm_minors_xa); /* * If the drm core fails to init for whatever reason, @@ -101,26 +101,23 @@ static struct drm_minor **drm_minor_get_slot(struct drm_device *dev, static void drm_minor_alloc_release(struct drm_device *dev, void *data) { struct drm_minor *minor = data; - unsigned long flags; WARN_ON(dev != minor->dev); put_device(minor->kdev); - if (minor->type == DRM_MINOR_ACCEL) { + if (minor->type == DRM_MINOR_ACCEL) accel_minor_remove(minor->index); - } else { - spin_lock_irqsave(&drm_minor_lock, flags); - idr_remove(&drm_minors_idr, minor->index); - spin_unlock_irqrestore(&drm_minor_lock, flags); - } + else + xa_erase(&drm_minors_xa, minor->index); } +#define DRM_MINOR_LIMIT(t) ({ typeof(t) _t = (t); XA_LIMIT(64 * _t, 64 * _t + 63); }) + static int drm_minor_alloc(struct drm_device *dev, enum drm_minor_type type) { struct drm_minor *minor; - unsigned long flags; - int r; + int index, r; minor = drmm_kzalloc(dev, sizeof(*minor), GFP_KERNEL); if (!minor) @@ -129,24 +126,17 @@ static int drm_minor_alloc(struct drm_device *dev, enum drm_minor_type type) minor->type = type; minor->dev = dev; - idr_preload(GFP_KERNEL); if (type == DRM_MINOR_ACCEL) { r = accel_minor_alloc(); + index = r; } else { - spin_lock_irqsave(&drm_minor_lock, flags); - r = idr_alloc(&drm_minors_idr, - NULL, - 64 * type, - 64 * (type + 1), - GFP_NOWAIT); - spin_unlock_irqrestore(&drm_minor_lock, flags); + r = xa_alloc(&drm_minors_xa, &index, NULL, DRM_MINOR_LIMIT(type), GFP_KERNEL); } - idr_preload_end(); if (r < 0) return r; - minor->index = r; + minor->index = index; r = drmm_add_action_or_reset(dev, drm_minor_alloc_release, minor); if (r) @@ -163,7 +153,7 @@ static int drm_minor_alloc(struct drm_device *dev, enum drm_minor_type type) static int drm_minor_register(struct drm_device *dev, enum drm_minor_type type) { struct drm_minor *minor; - unsigned long flags; + void *entry; int ret; DRM_DEBUG("\n"); @@ -190,9 +180,12 @@ static int drm_minor_register(struct drm_device *dev, enum drm_minor_type type) if (minor->type == DRM_MINOR_ACCEL) { accel_minor_replace(minor, minor->index); } else { - spin_lock_irqsave(&drm_minor_lock, flags); - idr_replace(&drm_minors_idr, minor, minor->index); - spin_unlock_irqrestore(&drm_minor_lock, flags); + entry = xa_store(&drm_minors_xa, minor->index, minor, GFP_KERNEL); + if (xa_is_err(entry)) { + ret = xa_err(entry); + goto err_debugfs; + } + WARN_ON(entry); [JZ] would WARN_ON(entry != minor)be better? } DRM_DEBUG("new minor registered %d\n", minor->index); @@ -206,20 +199,16 @@ static int drm_minor_register(struct drm_device *dev, enum drm_minor_type type) static void drm_minor_unregister(struct drm_device *dev, enum drm_minor_type type) { struct drm_minor *minor; - unsigned long flags; minor = *drm_minor_get_slot(dev, type); if (!minor || !device_is_registered(minor->kdev)) return; /* replace @minor with NULL so lookups will fail from now on */ - if (minor->type == DRM_MINOR_ACCEL) { + if (minor->type == DRM_MINOR_ACCEL) accel_minor_replace(NULL, minor->index); - } else { - spin_lock_irqsave(&drm_minor_lock, flags); - idr_replace(&drm_minors_idr, NULL, minor->index); - spin_unlock_
Re: [PATCH v6 3/4] drm: Expand max DRM device number to full MINORBITS
Hi Simon, Thanks! Yes, this kernel patch should work with latest libdrm. Best regards! James Zhu On 2023-08-23 06:53, Simon Ser wrote: On Tuesday, August 8th, 2023 at 17:04, James Zhu wrote: I have a MR for libdrm to support drm nodes type up to 2^MINORBITS nodes which can work with these patches, https://gitlab.freedesktop.org/mesa/drm/-/merge_requests/305 FWIW, this MR has been merged, so in theory this kernel patch should work fine with latest libdrm.
Re: [PATCH v6 3/4] drm: Expand max DRM device number to full MINORBITS
I would like if these kernel patches are accepted by everyone, If yes, when they can be upstream. I have a MR for libdrm to support drm nodes type up to 2^MINORBITS nodes which can work with these patches, https://gitlab.freedesktop.org/mesa/drm/-/merge_requests/305 Thanks! James On 2023-08-08 09:55, Christian König wrote: Am 28.07.23 um 16:22 schrieb Simon Ser: On Thursday, July 27th, 2023 at 14:01, Christian König wrote: We do need patches to stop trying to infer the node type from the minor in libdrm, though. Emil has suggested using sysfs, which we already do in a few places in libdrm. That sounds like a really good idea to me as well. But what do we do with DRM_MAX_MINOR? Change it or keep it and say apps should use drmGetDevices2() like Emil suggested? DRM_MAX_MINOR has been bumped to 64 now. With the new minor allocation scheme, DRM_MAX_MINOR is meaningless because there is no "max minor per type" concept anymore: the minor no longer contains the type. So I'd suggest leaving it as-is (so that old apps still continue to work on systems with < 64 devices like they do today) and mark it as deprecated. Sounds like a plan to me. Regards, Christian.
Re: [PATCH v6 3/4] drm: Expand max DRM device number to full MINORBITS
On 2023-07-24 17:14, Michał Winiarski wrote: Having a limit of 64 DRM devices is not good enough for modern world where we have multi-GPU servers, SR-IOV virtual functions and virtual devices used for testing. Let's utilize full minor range for DRM devices. To avoid regressing the existing userspace, we're still maintaining the numbering scheme where 0-63 is used for primary, 64-127 is reserved (formerly for control) and 128-191 is used for render. For minors >= 192, we're allocating minors dynamically on a first-come, first-served basis. [JZ] Hello Michal, Do you have libdrm patches for review together with this change? Especially to support static int drmGetMinorType(int major, int minor). Thanks and Best Regards! James Zhu Signed-off-by: Michał Winiarski --- drivers/gpu/drm/drm_drv.c | 12 1 file changed, 12 insertions(+) diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index 34b60196c443..c2c6e80e6b31 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -121,10 +121,19 @@ static void drm_minor_alloc_release(struct drm_device *dev, void *data) xa_erase(drm_minor_get_xa(minor->type), minor->index); } +/* + * DRM used to support 64 devices, for backwards compatibility we need to maintain the + * minor allocation scheme where minors 0-63 are primary nodes, 64-127 are control nodes, + * and 128-191 are render nodes. + * After reaching the limit, we're allocating minors dynamically - first-come, first-serve. + * Accel nodes are using a distinct major, so the minors are allocated in continuous 0-MAX + * range. + */ #define DRM_MINOR_LIMIT(t) ({ \ typeof(t) _t = (t); \ _t == DRM_MINOR_ACCEL ? XA_LIMIT(0, ACCEL_MAX_MINORS) : XA_LIMIT(64 * _t, 64 * _t + 63); \ }) +#define DRM_EXTENDED_MINOR_LIMIT XA_LIMIT(192, (1 << MINORBITS) - 1) static int drm_minor_alloc(struct drm_device *dev, enum drm_minor_type type) { @@ -140,6 +149,9 @@ static int drm_minor_alloc(struct drm_device *dev, enum drm_minor_type type) r = xa_alloc(drm_minor_get_xa(type), &minor->index, NULL, DRM_MINOR_LIMIT(type), GFP_KERNEL); + if (r == -EBUSY && (type == DRM_MINOR_PRIMARY || type == DRM_MINOR_RENDER)) + r = xa_alloc(&drm_minors_xa, &minor->index, +NULL, DRM_EXTENDED_MINOR_LIMIT, GFP_KERNEL); if (r < 0) return r;
Re: [PATCH 1/3] drm/drv: use enum drm_minor_type when appropriate
Hi Simon Thanks! Reviewed-by:JamesZhufortheseries.Best Regards! James Zhu On 2023-07-14 06:46, Simon Ser wrote: This makes it easier to figure out what the "type" variable can be set to when reading the implementation of these functions. Signed-off-by: Simon Ser Cc: Christian König Cc: James Zhu Cc: Marek Olšák Cc: Daniel Vetter --- drivers/gpu/drm/drm_drv.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index 12687dd9e1ac..3eda026ffac6 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -84,7 +84,7 @@ DEFINE_STATIC_SRCU(drm_unplug_srcu); */ static struct drm_minor **drm_minor_get_slot(struct drm_device *dev, -unsigned int type) +enum drm_minor_type type) { switch (type) { case DRM_MINOR_PRIMARY: @@ -116,7 +116,7 @@ static void drm_minor_alloc_release(struct drm_device *dev, void *data) } } -static int drm_minor_alloc(struct drm_device *dev, unsigned int type) +static int drm_minor_alloc(struct drm_device *dev, enum drm_minor_type type) { struct drm_minor *minor; unsigned long flags; @@ -160,7 +160,7 @@ static int drm_minor_alloc(struct drm_device *dev, unsigned int type) return 0; } -static int drm_minor_register(struct drm_device *dev, unsigned int type) +static int drm_minor_register(struct drm_device *dev, enum drm_minor_type type) { struct drm_minor *minor; unsigned long flags; @@ -203,7 +203,7 @@ static int drm_minor_register(struct drm_device *dev, unsigned int type) return ret; } -static void drm_minor_unregister(struct drm_device *dev, unsigned int type) +static void drm_minor_unregister(struct drm_device *dev, enum drm_minor_type type) { struct drm_minor *minor; unsigned long flags;
[PATCH] drm: support up to 128 drm devices
From: Christian König This makes room for up to 128 DRM devices. Signed-off-by: Christian König Signed-off-by: James Zhu --- drivers/gpu/drm/drm_drv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index 73b845a75d52..0d55c6f5 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -137,7 +137,7 @@ static int drm_minor_alloc(struct drm_device *dev, unsigned int type) r = idr_alloc(&drm_minors_idr, NULL, 64 * type, - 64 * (type + 1), + 64 * (type + 2), GFP_NOWAIT); spin_unlock_irqrestore(&drm_minor_lock, flags); } -- 2.34.1
[PATCH] drm: support up to 128 drm devices
From: Christian König This makes room for up to 128 DRM devices. Signed-off-by: Christian König Signed-off-by: James Zhu --- drivers/gpu/drm/drm_drv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index 73b845a75d52..0d55c6f5 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -137,7 +137,7 @@ static int drm_minor_alloc(struct drm_device *dev, unsigned int type) r = idr_alloc(&drm_minors_idr, NULL, 64 * type, - 64 * (type + 1), + 64 * (type + 2), GFP_NOWAIT); spin_unlock_irqrestore(&drm_minor_lock, flags); } -- 2.34.1
Re: linux-next: build failure after merge of the amdgpu tree
Hi Stephen, I saw https://gitlab.freedesktop.org/agd5f/linux/-/blob/amd-staging-drm-next/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c#349 should not have this error. Thanks! James Zhu On 2023-05-18 20:06, Stephen Rothwell wrote: Hi all, After merging the amdgpu tree, today's linux-next build (x86_64 allmodconfig) failed like this: drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c: In function 'amdgpu_ctx_init': drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c:348:26: error: 'fpriv' undeclared (first use in this function) 348 | ctx->ctx_mgr = &(fpriv->ctx_mgr); | ^ Caused by commit 2458393a4e98 ("drm/amdgpu: keep amdgpu_ctx_mgr in ctx structure") I have used the amdgpu tree from next-20230518 for today.
[PATCH v3 3/4] drm/sched: always keep selected ring sched list in ctx entity
Always keep selected ring sched list in ctx entity. Later entity->sched_list can always be used to track ring which is used in this ctx in amdgpu_ctx_fini_entity. v2: fixed typo v3. Update comments Signed-off-by: James Zhu --- drivers/gpu/drm/scheduler/sched_entity.c | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/scheduler/sched_entity.c b/drivers/gpu/drm/scheduler/sched_entity.c index f5595607995b..39dca9cb8e0d 100644 --- a/drivers/gpu/drm/scheduler/sched_entity.c +++ b/drivers/gpu/drm/scheduler/sched_entity.c @@ -71,7 +71,7 @@ int drm_sched_entity_init(struct drm_sched_entity *entity, entity->guilty = guilty; entity->num_sched_list = num_sched_list; entity->priority = priority; - entity->sched_list = num_sched_list > 1 ? sched_list : NULL; + entity->sched_list = sched_list; entity->last_scheduled = NULL; if(num_sched_list) @@ -453,7 +453,7 @@ void drm_sched_entity_select_rq(struct drm_sched_entity *entity) struct drm_sched_rq *rq; /* single possible engine and already selected */ - if (!entity->sched_list) + if (entity->num_sched_list <= 1) return; /* queue non-empty, stay on the same engine */ @@ -482,9 +482,6 @@ void drm_sched_entity_select_rq(struct drm_sched_entity *entity) entity->rq = rq; } spin_unlock(&entity->rq_lock); - - if (entity->num_sched_list == 1) - entity->sched_list = NULL; } /** -- 2.25.1
Re: [PATCH 1/4] drm/sched: returns struct drm_gpu_scheduler ** for drm_sched_pick_best
Yes, it is for NPI design. I will send out patches for review soon. Thanks! James On 2022-09-08 11:05 a.m., Andrey Grodzovsky wrote: So this is the real need of this patch-set, but this explanation doesn't appear anywhere in the description. It's always good to add a short 0 RFC patch which describes the intention of the patchset if the code is not self explanatory. And I still don't understand the need - i don't see anything in amdgpu_ctx_fini_entity regarding rings tracking ? Is it a new code you plan to add and not included in this patcheset ? Did i miss an earlier patch maybe ? Andrey On 2022-09-08 10:45, James Zhu wrote: To save lines is not the purpose. Also I want to use entity->sched_list to track ring which is used in this ctx in amdgpu_ctx_fini_entity Best Regards! James On 2022-09-08 10:38 a.m., Andrey Grodzovsky wrote: I guess it's an option but i don't really see what's the added value ? You saved a few lines in this patch but added a few lines in another. In total seems to me no to much difference ? Andrey On 2022-09-08 10:17, James Zhu wrote: Hi Andrey Basically this entire patch set are derived from patch [3/4]: entity->sched_list = num_sched_list > 1 ? sched_list : NULL; I think no special reason to treat single and multiple schedule list here. Best Regards! James On 2022-09-08 10:08 a.m., Andrey Grodzovsky wrote: What's the reason for this entire patch set ? Andrey On 2022-09-07 16:57, James Zhu wrote: drm_sched_pick_best returns struct drm_gpu_scheduler ** instead of struct drm_gpu_scheduler * Signed-off-by: James Zhu --- include/drm/gpu_scheduler.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h index 0fca8f38bee4..011f70a43397 100644 --- a/include/drm/gpu_scheduler.h +++ b/include/drm/gpu_scheduler.h @@ -529,7 +529,7 @@ void drm_sched_fence_finished(struct drm_sched_fence *fence); unsigned long drm_sched_suspend_timeout(struct drm_gpu_scheduler *sched); void drm_sched_resume_timeout(struct drm_gpu_scheduler *sched, unsigned long remaining); -struct drm_gpu_scheduler * +struct drm_gpu_scheduler ** drm_sched_pick_best(struct drm_gpu_scheduler **sched_list, unsigned int num_sched_list);
Re: [PATCH 1/4] drm/sched: returns struct drm_gpu_scheduler ** for drm_sched_pick_best
To save lines is not the purpose. Also I want to use entity->sched_list to track ring which is used in this ctx in amdgpu_ctx_fini_entity Best Regards! James On 2022-09-08 10:38 a.m., Andrey Grodzovsky wrote: I guess it's an option but i don't really see what's the added value ? You saved a few lines in this patch but added a few lines in another. In total seems to me no to much difference ? Andrey On 2022-09-08 10:17, James Zhu wrote: Hi Andrey Basically this entire patch set are derived from patch [3/4]: entity->sched_list = num_sched_list > 1 ? sched_list : NULL; I think no special reason to treat single and multiple schedule list here. Best Regards! James On 2022-09-08 10:08 a.m., Andrey Grodzovsky wrote: What's the reason for this entire patch set ? Andrey On 2022-09-07 16:57, James Zhu wrote: drm_sched_pick_best returns struct drm_gpu_scheduler ** instead of struct drm_gpu_scheduler * Signed-off-by: James Zhu --- include/drm/gpu_scheduler.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h index 0fca8f38bee4..011f70a43397 100644 --- a/include/drm/gpu_scheduler.h +++ b/include/drm/gpu_scheduler.h @@ -529,7 +529,7 @@ void drm_sched_fence_finished(struct drm_sched_fence *fence); unsigned long drm_sched_suspend_timeout(struct drm_gpu_scheduler *sched); void drm_sched_resume_timeout(struct drm_gpu_scheduler *sched, unsigned long remaining); -struct drm_gpu_scheduler * +struct drm_gpu_scheduler ** drm_sched_pick_best(struct drm_gpu_scheduler **sched_list, unsigned int num_sched_list);
Re: [PATCH 1/4] drm/sched: returns struct drm_gpu_scheduler ** for drm_sched_pick_best
Hi Andrey Basically this entire patch set are derived from patch [3/4]: entity->sched_list = num_sched_list > 1 ? sched_list : NULL; I think no special reason to treat single and multiple schedule list here. Best Regards! James On 2022-09-08 10:08 a.m., Andrey Grodzovsky wrote: What's the reason for this entire patch set ? Andrey On 2022-09-07 16:57, James Zhu wrote: drm_sched_pick_best returns struct drm_gpu_scheduler ** instead of struct drm_gpu_scheduler * Signed-off-by: James Zhu --- include/drm/gpu_scheduler.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h index 0fca8f38bee4..011f70a43397 100644 --- a/include/drm/gpu_scheduler.h +++ b/include/drm/gpu_scheduler.h @@ -529,7 +529,7 @@ void drm_sched_fence_finished(struct drm_sched_fence *fence); unsigned long drm_sched_suspend_timeout(struct drm_gpu_scheduler *sched); void drm_sched_resume_timeout(struct drm_gpu_scheduler *sched, unsigned long remaining); -struct drm_gpu_scheduler * +struct drm_gpu_scheduler ** drm_sched_pick_best(struct drm_gpu_scheduler **sched_list, unsigned int num_sched_list);
Re: [PATCH 3/4] drm/sched: always keep selecetd ring sched list in ctx entity
Hi Christian I need use entity->sched_list to track ring (ring = container_of(sched, struct amdgpu_ring, sched)) during amdgpu_ctx_fini_entity. I think change here to keep selected ring sched list in entity->sched_list won't change the original logic too much. Best Regards! James On 2022-09-08 2:15 a.m., Christian König wrote: Am 07.09.22 um 22:57 schrieb James Zhu: Always keep selecetd ring sched list in ctx entity. I have no idea what you are doing here, but this certainly doesn't make sense. Please explain a bit more. Thanks, Christian. Signed-off-by: James Zhu --- drivers/gpu/drm/scheduler/sched_entity.c | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/scheduler/sched_entity.c b/drivers/gpu/drm/scheduler/sched_entity.c index f5595607995b..39dca9cb8e0d 100644 --- a/drivers/gpu/drm/scheduler/sched_entity.c +++ b/drivers/gpu/drm/scheduler/sched_entity.c @@ -71,7 +71,7 @@ int drm_sched_entity_init(struct drm_sched_entity *entity, entity->guilty = guilty; entity->num_sched_list = num_sched_list; entity->priority = priority; - entity->sched_list = num_sched_list > 1 ? sched_list : NULL; + entity->sched_list = sched_list; entity->last_scheduled = NULL; if(num_sched_list) @@ -453,7 +453,7 @@ void drm_sched_entity_select_rq(struct drm_sched_entity *entity) struct drm_sched_rq *rq; /* single possible engine and already selected */ - if (!entity->sched_list) + if (entity->num_sched_list <= 1) return; /* queue non-empty, stay on the same engine */ @@ -482,9 +482,6 @@ void drm_sched_entity_select_rq(struct drm_sched_entity *entity) entity->rq = rq; } spin_unlock(&entity->rq_lock); - - if (entity->num_sched_list == 1) - entity->sched_list = NULL; } /**
[PATCH v2 3/4] drm/sched: always keep selected ring sched list in ctx entity
Always keep selected ring sched list in ctx entity. v2: fixed typo Signed-off-by: James Zhu --- drivers/gpu/drm/scheduler/sched_entity.c | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/scheduler/sched_entity.c b/drivers/gpu/drm/scheduler/sched_entity.c index f5595607995b..39dca9cb8e0d 100644 --- a/drivers/gpu/drm/scheduler/sched_entity.c +++ b/drivers/gpu/drm/scheduler/sched_entity.c @@ -71,7 +71,7 @@ int drm_sched_entity_init(struct drm_sched_entity *entity, entity->guilty = guilty; entity->num_sched_list = num_sched_list; entity->priority = priority; - entity->sched_list = num_sched_list > 1 ? sched_list : NULL; + entity->sched_list = sched_list; entity->last_scheduled = NULL; if(num_sched_list) @@ -453,7 +453,7 @@ void drm_sched_entity_select_rq(struct drm_sched_entity *entity) struct drm_sched_rq *rq; /* single possible engine and already selected */ - if (!entity->sched_list) + if (entity->num_sched_list <= 1) return; /* queue non-empty, stay on the same engine */ @@ -482,9 +482,6 @@ void drm_sched_entity_select_rq(struct drm_sched_entity *entity) entity->rq = rq; } spin_unlock(&entity->rq_lock); - - if (entity->num_sched_list == 1) - entity->sched_list = NULL; } /** -- 2.25.1
[PATCH 4/4] drm/amdgpu: update amdgpu_ctx_init_entity
update amdgpu_ctx_init_entity with new drm_sched_pick_best. Signed-off-by: James Zhu --- drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c index 6ea8980c8ad7..3a1cb0a70392 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c @@ -201,7 +201,7 @@ static ktime_t amdgpu_ctx_entity_time(struct amdgpu_ctx *ctx, static int amdgpu_ctx_init_entity(struct amdgpu_ctx *ctx, u32 hw_ip, const u32 ring) { - struct drm_gpu_scheduler **scheds = NULL, *sched = NULL; + struct drm_gpu_scheduler **scheds = NULL; struct amdgpu_device *adev = ctx->mgr->adev; struct amdgpu_ctx_entity *entity; enum drm_sched_priority drm_prio; @@ -230,8 +230,7 @@ static int amdgpu_ctx_init_entity(struct amdgpu_ctx *ctx, u32 hw_ip, hw_ip == AMDGPU_HW_IP_VCN_DEC || hw_ip == AMDGPU_HW_IP_UVD_ENC || hw_ip == AMDGPU_HW_IP_UVD) { - sched = drm_sched_pick_best(scheds, num_scheds); - scheds = &sched; + scheds = drm_sched_pick_best(scheds, num_scheds); num_scheds = 1; } -- 2.25.1
[PATCH 2/4] drm/sched: implement new drm_sched_pick_best
drm_sched_pick_best return best selecetd ring schedul list's address. Signed-off-by: James Zhu --- drivers/gpu/drm/scheduler/sched_entity.c | 2 +- drivers/gpu/drm/scheduler/sched_main.c | 14 -- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/scheduler/sched_entity.c b/drivers/gpu/drm/scheduler/sched_entity.c index 191c56064f19..f5595607995b 100644 --- a/drivers/gpu/drm/scheduler/sched_entity.c +++ b/drivers/gpu/drm/scheduler/sched_entity.c @@ -475,7 +475,7 @@ void drm_sched_entity_select_rq(struct drm_sched_entity *entity) return; spin_lock(&entity->rq_lock); - sched = drm_sched_pick_best(entity->sched_list, entity->num_sched_list); + sched = *drm_sched_pick_best(entity->sched_list, entity->num_sched_list); rq = sched ? &sched->sched_rq[entity->priority] : NULL; if (rq != entity->rq) { drm_sched_rq_remove_entity(entity->rq, entity); diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c index 68317d3a7a27..111277f6c53c 100644 --- a/drivers/gpu/drm/scheduler/sched_main.c +++ b/drivers/gpu/drm/scheduler/sched_main.c @@ -863,12 +863,12 @@ drm_sched_get_cleanup_job(struct drm_gpu_scheduler *sched) * Returns pointer of the sched with the least load or NULL if none of the * drm_gpu_schedulers are ready */ -struct drm_gpu_scheduler * +struct drm_gpu_scheduler ** drm_sched_pick_best(struct drm_gpu_scheduler **sched_list, unsigned int num_sched_list) { - struct drm_gpu_scheduler *sched, *picked_sched = NULL; - int i; + struct drm_gpu_scheduler *sched; + int i, picked_idx = -1; unsigned int min_score = UINT_MAX, num_score; for (i = 0; i < num_sched_list; ++i) { @@ -883,11 +883,13 @@ drm_sched_pick_best(struct drm_gpu_scheduler **sched_list, num_score = atomic_read(sched->score); if (num_score < min_score) { min_score = num_score; - picked_sched = sched; + picked_idx = i; } } - - return picked_sched; + if (picked_idx != -1) + return &(sched_list[picked_idx]); + else + return (struct drm_gpu_scheduler **)(NULL); } EXPORT_SYMBOL(drm_sched_pick_best); -- 2.25.1
[PATCH 3/4] drm/sched: always keep selecetd ring sched list in ctx entity
Always keep selecetd ring sched list in ctx entity. Signed-off-by: James Zhu --- drivers/gpu/drm/scheduler/sched_entity.c | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/scheduler/sched_entity.c b/drivers/gpu/drm/scheduler/sched_entity.c index f5595607995b..39dca9cb8e0d 100644 --- a/drivers/gpu/drm/scheduler/sched_entity.c +++ b/drivers/gpu/drm/scheduler/sched_entity.c @@ -71,7 +71,7 @@ int drm_sched_entity_init(struct drm_sched_entity *entity, entity->guilty = guilty; entity->num_sched_list = num_sched_list; entity->priority = priority; - entity->sched_list = num_sched_list > 1 ? sched_list : NULL; + entity->sched_list = sched_list; entity->last_scheduled = NULL; if(num_sched_list) @@ -453,7 +453,7 @@ void drm_sched_entity_select_rq(struct drm_sched_entity *entity) struct drm_sched_rq *rq; /* single possible engine and already selected */ - if (!entity->sched_list) + if (entity->num_sched_list <= 1) return; /* queue non-empty, stay on the same engine */ @@ -482,9 +482,6 @@ void drm_sched_entity_select_rq(struct drm_sched_entity *entity) entity->rq = rq; } spin_unlock(&entity->rq_lock); - - if (entity->num_sched_list == 1) - entity->sched_list = NULL; } /** -- 2.25.1
[PATCH 1/4] drm/sched: returns struct drm_gpu_scheduler ** for drm_sched_pick_best
drm_sched_pick_best returns struct drm_gpu_scheduler ** instead of struct drm_gpu_scheduler * Signed-off-by: James Zhu --- include/drm/gpu_scheduler.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h index 0fca8f38bee4..011f70a43397 100644 --- a/include/drm/gpu_scheduler.h +++ b/include/drm/gpu_scheduler.h @@ -529,7 +529,7 @@ void drm_sched_fence_finished(struct drm_sched_fence *fence); unsigned long drm_sched_suspend_timeout(struct drm_gpu_scheduler *sched); void drm_sched_resume_timeout(struct drm_gpu_scheduler *sched, unsigned long remaining); -struct drm_gpu_scheduler * +struct drm_gpu_scheduler ** drm_sched_pick_best(struct drm_gpu_scheduler **sched_list, unsigned int num_sched_list); -- 2.25.1
Re: [PATCH linux-next] drm/amdgpu/vcn: Remove unused assignment in vcn_v4_0_stop
ThispatchisReviewed-by:JamesZhu On 2022-08-15 3:00 a.m., Khalid Masum wrote: The value assigned from vcn_v4_0_stop_dbg_mode to r is overwritten before it can be used. Remove this assignment. Addresses-Coverity: 1504988 ("Unused value") Fixes: 8da1170a16e4 ("drm/amdgpu: add VCN4 ip block support") Signed-off-by: Khalid Masum --- drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c index ca14c3ef742e..80b8a2c66b36 100644 --- a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c @@ -1154,7 +1154,7 @@ static int vcn_v4_0_stop(struct amdgpu_device *adev) fw_shared->sq.queue_mode |= FW_QUEUE_DPG_HOLD_OFF; if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG) { - r = vcn_v4_0_stop_dpg_mode(adev, i); + vcn_v4_0_stop_dpg_mode(adev, i); continue; }
Re: [radeon-alex:drm-next-4.21-wip 10/27] drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c:1174 vcn_v1_0_stop_dpg_mode() error: uninitialized symbol 'ret_code'.
Hi Dan, Thanks! I send out the correct patch for review. Best Regards! James Zhu On 2018-10-16 09:56 AM, Dan Carpenter wrote: > tree: git://people.freedesktop.org/~agd5f/linux.git drm-next-4.21-wip > head: 6af94a9d0e185f48bef5cc1372f3ada89d003858 > commit: 15296db70619984157e60666da5da8994a66870e [10/27] drm/amdgpu/vcn:Add > ring W/R PTR check for VCN DPG mode stop > > smatch warnings: > drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c:1174 vcn_v1_0_stop_dpg_mode() error: > uninitialized symbol 'ret_code'. > > git remote add radeon-alex git://people.freedesktop.org/~agd5f/linux.git > git remote update radeon-alex > git checkout 15296db70619984157e60666da5da8994a66870e > vim +/ret_code +1174 drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c > > 88b5af70 Leo Liu 2016-12-28 1164 > 63e9bb1d James Zhu 2018-09-21 1165 static int vcn_v1_0_stop_dpg_mode(struct > amdgpu_device *adev) > 63e9bb1d James Zhu 2018-09-21 1166 { > 63e9bb1d James Zhu 2018-09-21 1167 int ret_code; > 63e9bb1d James Zhu 2018-09-21 1168 > b17c5249 James Zhu 2018-10-02 1169 /* Wait for power status to be > UVD_POWER_STATUS__UVD_POWER_STATUS_TILES_OFF */ > b17c5249 James Zhu 2018-10-02 1170 SOC15_WAIT_ON_RREG(UVD, 0, > mmUVD_POWER_STATUS, > b17c5249 James Zhu 2018-10-02 1171 > UVD_POWER_STATUS__UVD_POWER_STATUS_TILES_OFF, > 63e9bb1d James Zhu 2018-09-21 1172 > UVD_POWER_STATUS__UVD_POWER_STATUS_MASK, ret_code); > > ^^^^ > ret_code is only set on timeout. > > 63e9bb1d James Zhu 2018-09-21 1173 > 15296db7 James Zhu 2018-10-03 @1174 if (ret_code) { > > uninitialized on the success path. > > 15296db7 James Zhu 2018-10-03 1175 int tmp = RREG32_SOC15(UVD, 0, > mmUVD_RBC_RB_WPTR) & 0x7FFF; > 15296db7 James Zhu 2018-10-03 1176 /* wait for read ptr to be > equal to write ptr */ > 15296db7 James Zhu 2018-10-03 1177 SOC15_WAIT_ON_RREG(UVD, 0, > mmUVD_RBC_RB_RPTR, tmp, 0x, ret_code); > 15296db7 James Zhu 2018-10-03 1178 > 15296db7 James Zhu 2018-10-03 1179 SOC15_WAIT_ON_RREG(UVD, 0, > mmUVD_POWER_STATUS, > 15296db7 James Zhu 2018-10-03 1180 > UVD_POWER_STATUS__UVD_POWER_STATUS_TILES_OFF, > 15296db7 James Zhu 2018-10-03 1181 > UVD_POWER_STATUS__UVD_POWER_STATUS_MASK, ret_code); > 15296db7 James Zhu 2018-10-03 1182 } > 15296db7 James Zhu 2018-10-03 1183 > 63e9bb1d James Zhu 2018-09-21 1184 /* disable dynamic power gating mode */ > 63e9bb1d James Zhu 2018-09-21 1185 WREG32_P(SOC15_REG_OFFSET(UVD, 0, > mmUVD_POWER_STATUS), 0, > 63e9bb1d James Zhu 2018-09-21 1186 > ~UVD_POWER_STATUS__UVD_PG_MODE_MASK); > 63e9bb1d James Zhu 2018-09-21 1187 > 63e9bb1d James Zhu 2018-09-21 1188 return 0; > 63e9bb1d James Zhu 2018-09-21 1189 } > 63e9bb1d James Zhu 2018-09-21 1190 > > --- > 0-DAY kernel test infrastructureOpen Source Technology Center > https://lists.01.org/pipermail/kbuild-all Intel Corporation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/amdgpu: use first uvd instance to avoid clang build error
On 2018-06-17 04:52 AM, Stefan Agner wrote: Explicitly use the first uvd instance to avoid a build error when using clang 6: drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c:1148:52: error: expected ')' container_of(work, struct amdgpu_device, uvd.inst->idle_work.work); ^ drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c:1148:3: note: to match this '(' container_of(work, struct amdgpu_device, uvd.inst->idle_work.work); ^ ./include/linux/kernel.h:967:21: note: expanded from macro 'container_of' ((type *)(__mptr - offsetof(type, member))); }) ^ ./include/linux/stddef.h:17:32: note: expanded from macro 'offsetof' ^ ./include/linux/compiler-gcc.h:170:20: note: expanded from macro '__compiler_offsetof' __builtin_offsetof(a, b) ^ drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c:1147:24: error: initializing 'struct amdgpu_device *' with an expression of incompatible type 'void' struct amdgpu_device *adev = ^ 2 errors generated. Fixes: 10dd74eac4db ("drm/amdgpu/vg20:Restruct uvd.inst to support multiple instances") Cc: James Zhu Signed-off-by: Stefan Agner --- drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c index bcf68f80bbf0..a5888c44 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c @@ -1145,7 +1145,7 @@ int amdgpu_uvd_get_destroy_msg(struct amdgpu_ring *ring, uint32_t handle, static void amdgpu_uvd_idle_work_handler(struct work_struct *work) { struct amdgpu_device *adev = - container_of(work, struct amdgpu_device, uvd.inst->idle_work.work); + container_of(work, struct amdgpu_device, uvd.inst[0].idle_work.work); Hi Alex, If all instances share one idle work from hardware view currently and in the future , should we move struct delayed_work idle_work from struct amdgpu_uvd_inst to struct amdgpu_uvd? James unsigned fences = 0, i, j; for (i = 0; i < adev->uvd.num_uvd_inst; ++i) { ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH libdrm 1/2] add new uvd enc support check
Query hardware IP information to find out if there are uvd encode rings ready for use in kernel driver. Signed-off-by: James Zhu --- tests/amdgpu/uvd_enc_tests.c | 27 +-- 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/tests/amdgpu/uvd_enc_tests.c b/tests/amdgpu/uvd_enc_tests.c index 6c19f7b..7518103 100644 --- a/tests/amdgpu/uvd_enc_tests.c +++ b/tests/amdgpu/uvd_enc_tests.c @@ -79,6 +79,8 @@ static void amdgpu_cs_uvd_enc_session_init(void); static void amdgpu_cs_uvd_enc_encode(void); static void amdgpu_cs_uvd_enc_destroy(void); +static bool uvd_enc_support(void); + CU_TestInfo uvd_enc_tests[] = { { "UVD ENC create", amdgpu_cs_uvd_enc_create }, { "UVD ENC session init", amdgpu_cs_uvd_enc_session_init }, @@ -98,7 +100,7 @@ int suite_uvd_enc_tests_init(void) family_id = device_handle->info.family_id; - if (family_id < AMDGPU_FAMILY_AI || family_id >= AMDGPU_FAMILY_RV) { + if (!uvd_enc_support()) { printf("\n\nThe ASIC NOT support UVD ENC, all sub-tests will pass\n"); return CUE_SUCCESS; } @@ -121,7 +123,7 @@ int suite_uvd_enc_tests_clean(void) { int r; - if (family_id < AMDGPU_FAMILY_AI || family_id >= AMDGPU_FAMILY_RV) { + if (!uvd_enc_support()) { r = amdgpu_device_deinitialize(device_handle); if (r) @@ -238,11 +240,24 @@ static void free_resource(struct amdgpu_uvd_enc_bo *uvd_enc_bo) memset(uvd_enc_bo, 0, sizeof(*uvd_enc_bo)); } +static bool uvd_enc_support(void) +{ + int r; + struct drm_amdgpu_info_hw_ip info; + + r = amdgpu_query_hw_ip_info(device_handle, AMDGPU_HW_IP_UVD_ENC, 0, &info); + + if (r) + return false; + else + return (info.available_rings?true:false); +} + static void amdgpu_cs_uvd_enc_create(void) { int len, r; - if (family_id < AMDGPU_FAMILY_AI || family_id >= AMDGPU_FAMILY_RV) + if (!uvd_enc_support()) return; enc.width = 160; @@ -281,7 +296,7 @@ static void amdgpu_cs_uvd_enc_session_init(void) { int len, r; - if (family_id < AMDGPU_FAMILY_AI || family_id >= AMDGPU_FAMILY_RV) + if (!uvd_enc_support()) return; len = 0; @@ -339,7 +354,7 @@ static void amdgpu_cs_uvd_enc_encode(void) vbuf_size = ALIGN(enc.width, align) * ALIGN(enc.height, 16) * 1.5; cpb_size = vbuf_size * 10; - if (family_id < AMDGPU_FAMILY_AI || family_id >= AMDGPU_FAMILY_RV) + if (!uvd_enc_support()) return; num_resources = 0; @@ -472,7 +487,7 @@ static void amdgpu_cs_uvd_enc_destroy(void) struct amdgpu_uvd_enc_bo sw_ctx; int len, r; - if (family_id < AMDGPU_FAMILY_AI || family_id >= AMDGPU_FAMILY_RV) + if (!uvd_enc_support()) return; num_resources = 0; -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH libdrm v2 1/2] tests/amdgpu: add new uvd enc support check
Query hardware IP information to find out if there are uvd encode rings ready for use in kernel driver. Signed-off-by: James Zhu --- tests/amdgpu/uvd_enc_tests.c | 27 +-- 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/tests/amdgpu/uvd_enc_tests.c b/tests/amdgpu/uvd_enc_tests.c index 6c19f7b..7518103 100644 --- a/tests/amdgpu/uvd_enc_tests.c +++ b/tests/amdgpu/uvd_enc_tests.c @@ -79,6 +79,8 @@ static void amdgpu_cs_uvd_enc_session_init(void); static void amdgpu_cs_uvd_enc_encode(void); static void amdgpu_cs_uvd_enc_destroy(void); +static bool uvd_enc_support(void); + CU_TestInfo uvd_enc_tests[] = { { "UVD ENC create", amdgpu_cs_uvd_enc_create }, { "UVD ENC session init", amdgpu_cs_uvd_enc_session_init }, @@ -98,7 +100,7 @@ int suite_uvd_enc_tests_init(void) family_id = device_handle->info.family_id; - if (family_id < AMDGPU_FAMILY_AI || family_id >= AMDGPU_FAMILY_RV) { + if (!uvd_enc_support()) { printf("\n\nThe ASIC NOT support UVD ENC, all sub-tests will pass\n"); return CUE_SUCCESS; } @@ -121,7 +123,7 @@ int suite_uvd_enc_tests_clean(void) { int r; - if (family_id < AMDGPU_FAMILY_AI || family_id >= AMDGPU_FAMILY_RV) { + if (!uvd_enc_support()) { r = amdgpu_device_deinitialize(device_handle); if (r) @@ -238,11 +240,24 @@ static void free_resource(struct amdgpu_uvd_enc_bo *uvd_enc_bo) memset(uvd_enc_bo, 0, sizeof(*uvd_enc_bo)); } +static bool uvd_enc_support(void) +{ + int r; + struct drm_amdgpu_info_hw_ip info; + + r = amdgpu_query_hw_ip_info(device_handle, AMDGPU_HW_IP_UVD_ENC, 0, &info); + + if (r) + return false; + else + return (info.available_rings?true:false); +} + static void amdgpu_cs_uvd_enc_create(void) { int len, r; - if (family_id < AMDGPU_FAMILY_AI || family_id >= AMDGPU_FAMILY_RV) + if (!uvd_enc_support()) return; enc.width = 160; @@ -281,7 +296,7 @@ static void amdgpu_cs_uvd_enc_session_init(void) { int len, r; - if (family_id < AMDGPU_FAMILY_AI || family_id >= AMDGPU_FAMILY_RV) + if (!uvd_enc_support()) return; len = 0; @@ -339,7 +354,7 @@ static void amdgpu_cs_uvd_enc_encode(void) vbuf_size = ALIGN(enc.width, align) * ALIGN(enc.height, 16) * 1.5; cpb_size = vbuf_size * 10; - if (family_id < AMDGPU_FAMILY_AI || family_id >= AMDGPU_FAMILY_RV) + if (!uvd_enc_support()) return; num_resources = 0; @@ -472,7 +487,7 @@ static void amdgpu_cs_uvd_enc_destroy(void) struct amdgpu_uvd_enc_bo sw_ctx; int len, r; - if (family_id < AMDGPU_FAMILY_AI || family_id >= AMDGPU_FAMILY_RV) + if (!uvd_enc_support()) return; num_resources = 0; -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH libdrm v2 2/2] tests/amdgpu: fix uvd enc data corruption issue
Hi Leo, Sure, I will reset 0 in header file Thanks! James Zhu On 2017-10-05 11:39 AM, Leo Liu wrote: On 10/05/2017 11:24 AM, James Zhu wrote: In uvd encode parameter package, parameters input_pic_luma_pitch and input_pic_chroma_pitch should be picture width align with hardware alignment. The hardware alignment is 16 for amdgpu family earlier than AMDGPU_FAMILY_AI, and 256 for later than and including AMDGPU_FAMILY_AI. Signed-off-by: James Zhu --- tests/amdgpu/uvd_enc_tests.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/amdgpu/uvd_enc_tests.c b/tests/amdgpu/uvd_enc_tests.c index 7518103..bbda131 100644 --- a/tests/amdgpu/uvd_enc_tests.c +++ b/tests/amdgpu/uvd_enc_tests.c @@ -272,7 +272,7 @@ static void amdgpu_cs_uvd_enc_create(void) static void check_result(struct amdgpu_uvd_enc *enc) { uint64_t sum; - uint32_t s = 26382; + uint32_t s = 175602; uint32_t *ptr, size; int i, j, r; @@ -463,6 +463,8 @@ static void amdgpu_cs_uvd_enc_encode(void) ib_cpu[len++] = chroma_offset >> 32; ib_cpu[len++] = chroma_offset; memcpy((ib_cpu + len), uve_encode_param, sizeof(uve_encode_param)); + ib_cpu[len] = ALIGN(enc.width, align); + ib_cpu[len + 1] = ALIGN(enc.width, align); Since here we override the pitch value based on below from uve_ib.h. static const uint32_t uve_encode_param[] = { 0x00a0, 0x0080, We'd better to reset them to 0 from the header file, since we don't want to leave the incorrect value there. With that fixed, the series is Reviewed-by: Leo Liu len += sizeof(uve_encode_param) / 4; memcpy((ib_cpu + len), uve_op_speed_enc_mode, sizeof(uve_op_speed_enc_mode)); ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH libdrm v2 2/2] tests/amdgpu: fix uvd enc data corruption issue
In uvd encode parameter package, parameters input_pic_luma_pitch and input_pic_chroma_pitch should be picture width align with hardware alignment. The hardware alignment is 16 for amdgpu family earlier than AMDGPU_FAMILY_AI, and 256 for later than and including AMDGPU_FAMILY_AI. Signed-off-by: James Zhu --- tests/amdgpu/uvd_enc_tests.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/amdgpu/uvd_enc_tests.c b/tests/amdgpu/uvd_enc_tests.c index 7518103..bbda131 100644 --- a/tests/amdgpu/uvd_enc_tests.c +++ b/tests/amdgpu/uvd_enc_tests.c @@ -272,7 +272,7 @@ static void amdgpu_cs_uvd_enc_create(void) static void check_result(struct amdgpu_uvd_enc *enc) { uint64_t sum; - uint32_t s = 26382; + uint32_t s = 175602; uint32_t *ptr, size; int i, j, r; @@ -463,6 +463,8 @@ static void amdgpu_cs_uvd_enc_encode(void) ib_cpu[len++] = chroma_offset >> 32; ib_cpu[len++] = chroma_offset; memcpy((ib_cpu + len), uve_encode_param, sizeof(uve_encode_param)); + ib_cpu[len] = ALIGN(enc.width, align); + ib_cpu[len + 1] = ALIGN(enc.width, align); len += sizeof(uve_encode_param) / 4; memcpy((ib_cpu + len), uve_op_speed_enc_mode, sizeof(uve_op_speed_enc_mode)); -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH libdrm 2/2] fix uvd enc data corruption issue
In uvd encode parameter package, parameters input_pic_luma_pitch and input_pic_chroma_pitch should be picture width align with hardware alignment. The hardware alignment is 16 for amdgpu family earlier than AMDGPU_FAMILY_AI, and 256 for later than and including AMDGPU_FAMILY_AI. Signed-off-by: James Zhu --- tests/amdgpu/uvd_enc_tests.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/amdgpu/uvd_enc_tests.c b/tests/amdgpu/uvd_enc_tests.c index 7518103..bbda131 100644 --- a/tests/amdgpu/uvd_enc_tests.c +++ b/tests/amdgpu/uvd_enc_tests.c @@ -272,7 +272,7 @@ static void amdgpu_cs_uvd_enc_create(void) static void check_result(struct amdgpu_uvd_enc *enc) { uint64_t sum; - uint32_t s = 26382; + uint32_t s = 175602; uint32_t *ptr, size; int i, j, r; @@ -463,6 +463,8 @@ static void amdgpu_cs_uvd_enc_encode(void) ib_cpu[len++] = chroma_offset >> 32; ib_cpu[len++] = chroma_offset; memcpy((ib_cpu + len), uve_encode_param, sizeof(uve_encode_param)); + ib_cpu[len] = ALIGN(enc.width, align); + ib_cpu[len + 1] = ALIGN(enc.width, align); len += sizeof(uve_encode_param) / 4; memcpy((ib_cpu + len), uve_op_speed_enc_mode, sizeof(uve_op_speed_enc_mode)); -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel