| Message ID | 20231017035656.8211-1-luben.tuikov@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] drm/amdgpu: Unset context priority is now invalid | expand |
On Tue, Oct 17, 2023 at 12:52 AM Luben Tuikov <luben.tuikov@amd.com> wrote: > > A context priority value of AMD_CTX_PRIORITY_UNSET is now invalid--instead of > carrying it around and passing it to the Direct Rendering Manager--and it > becomes AMD_CTX_PRIORITY_NORMAL in amdgpu_ctx_ioctl(), the gateway to context > creation. > > Cc: Alex Deucher <Alexander.Deucher@amd.com> > Cc: Christian König <christian.koenig@amd.com> > Signed-off-by: Luben Tuikov <luben.tuikov@amd.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c > index 0dc9c655c4fbdb..092962b93064fc 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c > @@ -47,7 +47,6 @@ const unsigned int amdgpu_ctx_num_entities[AMDGPU_HW_IP_NUM] = { > bool amdgpu_ctx_priority_is_valid(int32_t ctx_prio) > { > switch (ctx_prio) { > - case AMDGPU_CTX_PRIORITY_UNSET: > case AMDGPU_CTX_PRIORITY_VERY_LOW: > case AMDGPU_CTX_PRIORITY_LOW: > case AMDGPU_CTX_PRIORITY_NORMAL: > @@ -55,6 +54,7 @@ bool amdgpu_ctx_priority_is_valid(int32_t ctx_prio) > case AMDGPU_CTX_PRIORITY_VERY_HIGH: > return true; > default: > + case AMDGPU_CTX_PRIORITY_UNSET: > return false; I don't recall if any userspace uses this, but this would break userspace if it does. Alex > } > } > > base-commit: dc9b2e683bcba017588b9aaad80f442ad004a48f > -- > 2.42.0 >
On 2023-10-17 09:22, Alex Deucher wrote: > On Tue, Oct 17, 2023 at 12:52 AM Luben Tuikov <luben.tuikov@amd.com> wrote: >> >> A context priority value of AMD_CTX_PRIORITY_UNSET is now invalid--instead of >> carrying it around and passing it to the Direct Rendering Manager--and it >> becomes AMD_CTX_PRIORITY_NORMAL in amdgpu_ctx_ioctl(), the gateway to context >> creation. >> >> Cc: Alex Deucher <Alexander.Deucher@amd.com> >> Cc: Christian König <christian.koenig@amd.com> >> Signed-off-by: Luben Tuikov <luben.tuikov@amd.com> >> --- >> drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c >> index 0dc9c655c4fbdb..092962b93064fc 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c >> @@ -47,7 +47,6 @@ const unsigned int amdgpu_ctx_num_entities[AMDGPU_HW_IP_NUM] = { >> bool amdgpu_ctx_priority_is_valid(int32_t ctx_prio) >> { >> switch (ctx_prio) { >> - case AMDGPU_CTX_PRIORITY_UNSET: >> case AMDGPU_CTX_PRIORITY_VERY_LOW: >> case AMDGPU_CTX_PRIORITY_LOW: >> case AMDGPU_CTX_PRIORITY_NORMAL: >> @@ -55,6 +54,7 @@ bool amdgpu_ctx_priority_is_valid(int32_t ctx_prio) >> case AMDGPU_CTX_PRIORITY_VERY_HIGH: >> return true; >> default: >> + case AMDGPU_CTX_PRIORITY_UNSET: >> return false; > > I don't recall if any userspace uses this, but this would break > userspace if it does. This is shielded from user space in the following manner, 1) amdgpu_ctx_priority_is_valid() is called from amdgpu_ctx_ioctl() and if amdgpu_ctx_priority_is_valid() returns false, we set the priority to NORMAL. See the 2nd patch. 2) It is also called from amdgpu_ctx_priority_permit(), which is called from amdgpu_ctx_init() which is called from amdgpu_ctx_alloc() which is called from amdgpu_ctx_ioctl(), _after_ the call described above, and thus priority is now NORMAL. Plus I'm typing this on a running system with 6.6.0 + those two patches. User space can send us down UNSET, but we set it to NORMAL. Can I get an R-B? > > Alex > >> } >> } >> >> base-commit: dc9b2e683bcba017588b9aaad80f442ad004a48f >> -- >> 2.42.0 >>
On Tue, Oct 17, 2023 at 10:27 AM Luben Tuikov <luben.tuikov@amd.com> wrote: > > On 2023-10-17 09:22, Alex Deucher wrote: > > On Tue, Oct 17, 2023 at 12:52 AM Luben Tuikov <luben.tuikov@amd.com> wrote: > >> > >> A context priority value of AMD_CTX_PRIORITY_UNSET is now invalid--instead of > >> carrying it around and passing it to the Direct Rendering Manager--and it > >> becomes AMD_CTX_PRIORITY_NORMAL in amdgpu_ctx_ioctl(), the gateway to context > >> creation. > >> > >> Cc: Alex Deucher <Alexander.Deucher@amd.com> > >> Cc: Christian König <christian.koenig@amd.com> > >> Signed-off-by: Luben Tuikov <luben.tuikov@amd.com> > >> --- > >> drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c > >> index 0dc9c655c4fbdb..092962b93064fc 100644 > >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c > >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c > >> @@ -47,7 +47,6 @@ const unsigned int amdgpu_ctx_num_entities[AMDGPU_HW_IP_NUM] = { > >> bool amdgpu_ctx_priority_is_valid(int32_t ctx_prio) > >> { > >> switch (ctx_prio) { > >> - case AMDGPU_CTX_PRIORITY_UNSET: > >> case AMDGPU_CTX_PRIORITY_VERY_LOW: > >> case AMDGPU_CTX_PRIORITY_LOW: > >> case AMDGPU_CTX_PRIORITY_NORMAL: > >> @@ -55,6 +54,7 @@ bool amdgpu_ctx_priority_is_valid(int32_t ctx_prio) > >> case AMDGPU_CTX_PRIORITY_VERY_HIGH: > >> return true; > >> default: > >> + case AMDGPU_CTX_PRIORITY_UNSET: > >> return false; > > > > I don't recall if any userspace uses this, but this would break > > userspace if it does. > > This is shielded from user space in the following manner, > 1) amdgpu_ctx_priority_is_valid() is called from amdgpu_ctx_ioctl() and > if amdgpu_ctx_priority_is_valid() returns false, we set the priority to NORMAL. > See the 2nd patch. Ah, I see. Thanks. Series is: Acked-by: Alex Deucher <alexander.deucher@amd.com> > 2) It is also called from amdgpu_ctx_priority_permit(), which is called > from amdgpu_ctx_init() which is called from amdgpu_ctx_alloc() which > is called from amdgpu_ctx_ioctl(), _after_ the call described above, > and thus priority is now NORMAL. > > Plus I'm typing this on a running system with 6.6.0 + those two patches. > > User space can send us down UNSET, but we set it to NORMAL. > > Can I get an R-B? > > > > > Alex > > > >> } > >> } > >> > >> base-commit: dc9b2e683bcba017588b9aaad80f442ad004a48f > >> -- > >> 2.42.0 > >> > > -- > Regards, > Luben >
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c index 0dc9c655c4fbdb..092962b93064fc 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c @@ -47,7 +47,6 @@ const unsigned int amdgpu_ctx_num_entities[AMDGPU_HW_IP_NUM] = { bool amdgpu_ctx_priority_is_valid(int32_t ctx_prio) { switch (ctx_prio) { - case AMDGPU_CTX_PRIORITY_UNSET: case AMDGPU_CTX_PRIORITY_VERY_LOW: case AMDGPU_CTX_PRIORITY_LOW: case AMDGPU_CTX_PRIORITY_NORMAL: @@ -55,6 +54,7 @@ bool amdgpu_ctx_priority_is_valid(int32_t ctx_prio) case AMDGPU_CTX_PRIORITY_VERY_HIGH: return true; default: + case AMDGPU_CTX_PRIORITY_UNSET: return false; } }
A context priority value of AMD_CTX_PRIORITY_UNSET is now invalid--instead of carrying it around and passing it to the Direct Rendering Manager--and it becomes AMD_CTX_PRIORITY_NORMAL in amdgpu_ctx_ioctl(), the gateway to context creation. Cc: Alex Deucher <Alexander.Deucher@amd.com> Cc: Christian König <christian.koenig@amd.com> Signed-off-by: Luben Tuikov <luben.tuikov@amd.com> --- drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) base-commit: dc9b2e683bcba017588b9aaad80f442ad004a48f