| Message ID | 20250415184318.2465197-2-sunil.khatri@amd.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v3,1/4] drm: add function drm_file_err to print proc information too | expand |
On 15/04/2025 19:43, Sunil Khatri wrote: > drm_file will be used in usermode queues code to > enable better process information in logging and hence > add drm_file part of the userq_mgr struct. > > update the drm_file pointer in userq_mgr for each > amdgpu_driver_open_kms. > > Signed-off-by: Sunil Khatri <sunil.khatri@amd.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 1 + > drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.h | 1 + > 2 files changed, 2 insertions(+) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > index 3d319687c1c9..3de3071d66ee 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > @@ -1436,6 +1436,7 @@ int amdgpu_driver_open_kms(struct drm_device *dev, struct drm_file *file_priv) > > amdgpu_ctx_mgr_init(&fpriv->ctx_mgr, adev); > > + fpriv->userq_mgr.file = file_priv; > r = amdgpu_userq_mgr_init(&fpriv->userq_mgr, adev); It's a bit of a layering violation since amdgpu_userq_mgr_init() is the place which otherwise initialises fpriv->user_mgr. One day someome might put a memset in there for example. Anyway, I think it would be nicer if you passed fpriv to that function. Potentially instead of adev. Looks like that would be cleaner "design". Regards, Tvrtko > if (r) > DRM_WARN("Can't setup usermode queues, use legacy workload submission only\n"); > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.h > index 381b9c6f0573..fe51a45f7ee4 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.h > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.h > @@ -77,6 +77,7 @@ struct amdgpu_userq_mgr { > struct amdgpu_device *adev; > struct delayed_work resume_work; > struct list_head list; > + struct drm_file *file; > }; > > struct amdgpu_db_info {
On 4/16/2025 12:59 PM, Tvrtko Ursulin wrote: > > On 15/04/2025 19:43, Sunil Khatri wrote: >> drm_file will be used in usermode queues code to >> enable better process information in logging and hence >> add drm_file part of the userq_mgr struct. >> >> update the drm_file pointer in userq_mgr for each >> amdgpu_driver_open_kms. >> >> Signed-off-by: Sunil Khatri <sunil.khatri@amd.com> >> --- >> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 1 + >> drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.h | 1 + >> 2 files changed, 2 insertions(+) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c >> index 3d319687c1c9..3de3071d66ee 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c >> @@ -1436,6 +1436,7 @@ int amdgpu_driver_open_kms(struct drm_device >> *dev, struct drm_file *file_priv) >> amdgpu_ctx_mgr_init(&fpriv->ctx_mgr, adev); >> + fpriv->userq_mgr.file = file_priv; >> r = amdgpu_userq_mgr_init(&fpriv->userq_mgr, adev); > > It's a bit of a layering violation since amdgpu_userq_mgr_init() is > the place which otherwise initialises fpriv->user_mgr. One day someome > might put a memset in there for example. Anyway, I think it would be > nicer if you passed fpriv to that function. Potentially instead of > adev. Looks like that would be cleaner "design". > I agree totally this should be inside amdgpu_userq_mgr_init with fpriv passed to function. But i guess whoever wrote it in first place thought to make it same as done in a line above fot ctx_mgr. Once we have these patches merge i will push these fixes separately. Regards Sunil Khatri > Regards, > > Tvrtko > >> if (r) >> DRM_WARN("Can't setup usermode queues, use legacy workload >> submission only\n"); >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.h >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.h >> index 381b9c6f0573..fe51a45f7ee4 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.h >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.h >> @@ -77,6 +77,7 @@ struct amdgpu_userq_mgr { >> struct amdgpu_device *adev; >> struct delayed_work resume_work; >> struct list_head list; >> + struct drm_file *file; >> }; >> struct amdgpu_db_info { >
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c index 3d319687c1c9..3de3071d66ee 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c @@ -1436,6 +1436,7 @@ int amdgpu_driver_open_kms(struct drm_device *dev, struct drm_file *file_priv) amdgpu_ctx_mgr_init(&fpriv->ctx_mgr, adev); + fpriv->userq_mgr.file = file_priv; r = amdgpu_userq_mgr_init(&fpriv->userq_mgr, adev); if (r) DRM_WARN("Can't setup usermode queues, use legacy workload submission only\n"); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.h index 381b9c6f0573..fe51a45f7ee4 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.h @@ -77,6 +77,7 @@ struct amdgpu_userq_mgr { struct amdgpu_device *adev; struct delayed_work resume_work; struct list_head list; + struct drm_file *file; }; struct amdgpu_db_info {
drm_file will be used in usermode queues code to enable better process information in logging and hence add drm_file part of the userq_mgr struct. update the drm_file pointer in userq_mgr for each amdgpu_driver_open_kms. Signed-off-by: Sunil Khatri <sunil.khatri@amd.com> --- drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 1 + drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.h | 1 + 2 files changed, 2 insertions(+)