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(_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(_minor_lock, flags);
- idr_remove(_minors_idr, minor->index);
- spin_unlock_irqrestore(_minor_lock, flags);
- }
+ else
+ xa_erase(_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(_minor_lock, flags);
- r = idr_alloc(_minors_idr,
- NULL,
- 64 * type,
- 64 * (type + 1),
- GFP_NOWAIT);
- spin_unlock_irqrestore(_minor_lock, flags);
+ r = xa_alloc(_minors_xa, , 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(_minor_lock, flags);
- idr_replace(_minors_idr, minor, minor->index);
- spin_unlock_irqrestore(_minor_lock, flags);
+ entry = xa_store(_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(_minors_xa, >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 = *drm_minor_get_slot(dev, type);
if (!minor || !device_is_registered(minor->kdev))
return;
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(_minor_lock, flags);
- idr_remove(_minors_idr, minor->index);
- spin_unlock_irqrestore(_minor_lock, flags);
- }
+ else
+ xa_erase(_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(_minor_lock, flags);
- r = idr_alloc(_minors_idr,
- NULL,
- 64 * type,
- 64 * (type + 1),
- GFP_NOWAIT);
- spin_unlock_irqrestore(_minor_lock, flags);
+ r = xa_alloc(_minors_xa, , 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(_minor_lock, flags);
- idr_replace(_minors_idr, minor, minor->index);
- spin_unlock_irqrestore(_minor_lock, flags);
+ entry = xa_store(_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(_minor_lock, flags);
- idr_replace(_minors_idr, NULL, minor->index);
- spin_unlock_irqrestore(_minor_lock, flags);
- }
+ else
+
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), >index,
NULL, DRM_MINOR_LIMIT(type), GFP_KERNEL);
+ if (r == -EBUSY && (type == DRM_MINOR_PRIMARY || type ==
DRM_MINOR_RENDER))
+ r = xa_alloc(_minors_xa, >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(_minors_idr, NULL, 64 * type, - 64 * (type + 1), + 64 * (type + 2), GFP_NOWAIT); spin_unlock_irqrestore(_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(_minors_idr, NULL, 64 * type, - 64 * (type + 1), + 64 * (type + 2), GFP_NOWAIT); spin_unlock_irqrestore(_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(>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(>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(>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 =
+ 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(>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_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(>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 <james@amd.com>
---
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,
);
+
+ 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 <james@amd.com>
---
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,
);
+
+ 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 <james@amd.com>
---
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 <leo@amd.com>
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 <james@amd.com>
---
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 <james@amd.com>
---
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
