| Message ID | 20230714104557.518457-3-contact@emersion.fr (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/3] drm/drv: use enum drm_minor_type when appropriate | expand |
Hi Simon Am 14.07.23 um 12:46 schrieb Simon Ser: > This entry should never be used by the kernel. Record the historical > context in a comment. > > Signed-off-by: Simon Ser <contact@emersion.fr> > Cc: Christian König <christian.koenig@amd.com> > Cc: James Zhu <James.Zhu@amd.com> > Cc: Marek Olšák <marek.olsak@amd.com> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > --- > include/drm/drm_file.h | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h > index 010239392adf..a23cc2f6163f 100644 > --- a/include/drm/drm_file.h > +++ b/include/drm/drm_file.h > @@ -53,12 +53,14 @@ struct file; > /* Note that the values of this enum are ABI (it determines > * /dev/dri/renderD* numbers). > * > + * There used to be a DRM_MINOR_CONTROL = 1 entry, but such nodes were never > + * exposed. Still, some user-space has logic to handle them. > + * > * Setting DRM_MINOR_ACCEL to 32 gives enough space for more drm minors to > * be implemented before we hit any future > */ > enum drm_minor_type { > DRM_MINOR_PRIMARY = 0, > - DRM_MINOR_CONTROL = 1, Maybe rather leave this line in and comment it with "// obsolete". Otherwise someone might accidentally use 1 for something. In any case Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de> for the whole series. Best regards Thomas > DRM_MINOR_RENDER = 2, > DRM_MINOR_ACCEL = 32, > };
On Friday, July 14th, 2023 at 16:28, Thomas Zimmermann <tzimmermann@suse.de> wrote: > Hi Simon > > Am 14.07.23 um 12:46 schrieb Simon Ser: > > This entry should never be used by the kernel. Record the historical > > context in a comment. > > > > Signed-off-by: Simon Ser <contact@emersion.fr> > > Cc: Christian König <christian.koenig@amd.com> > > Cc: James Zhu <James.Zhu@amd.com> > > Cc: Marek Olšák <marek.olsak@amd.com> > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > > --- > > include/drm/drm_file.h | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h > > index 010239392adf..a23cc2f6163f 100644 > > --- a/include/drm/drm_file.h > > +++ b/include/drm/drm_file.h > > @@ -53,12 +53,14 @@ struct file; > > /* Note that the values of this enum are ABI (it determines > > * /dev/dri/renderD* numbers). > > * > > + * There used to be a DRM_MINOR_CONTROL = 1 entry, but such nodes were never > > + * exposed. Still, some user-space has logic to handle them. > > + * > > * Setting DRM_MINOR_ACCEL to 32 gives enough space for more drm minors to > > * be implemented before we hit any future > > */ > > enum drm_minor_type { > > DRM_MINOR_PRIMARY = 0, > > - DRM_MINOR_CONTROL = 1, > > Maybe rather leave this line in and comment it with "// obsolete". > Otherwise someone might accidentally use 1 for something. Yeah... That's why I added the comment. But maybe better to leave the entry indeed, even if we risk someone accidentally using it. > In any case > > Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de> > > for the whole series. Thanks for the review!
Hi Am 14.07.23 um 16:37 schrieb Simon Ser: > On Friday, July 14th, 2023 at 16:28, Thomas Zimmermann <tzimmermann@suse.de> wrote: > >> Hi Simon >> >> Am 14.07.23 um 12:46 schrieb Simon Ser: >>> This entry should never be used by the kernel. Record the historical >>> context in a comment. >>> >>> Signed-off-by: Simon Ser <contact@emersion.fr> >>> Cc: Christian König <christian.koenig@amd.com> >>> Cc: James Zhu <James.Zhu@amd.com> >>> Cc: Marek Olšák <marek.olsak@amd.com> >>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> >>> --- >>> include/drm/drm_file.h | 4 +++- >>> 1 file changed, 3 insertions(+), 1 deletion(-) >>> >>> diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h >>> index 010239392adf..a23cc2f6163f 100644 >>> --- a/include/drm/drm_file.h >>> +++ b/include/drm/drm_file.h >>> @@ -53,12 +53,14 @@ struct file; >>> /* Note that the values of this enum are ABI (it determines >>> * /dev/dri/renderD* numbers). >>> * >>> + * There used to be a DRM_MINOR_CONTROL = 1 entry, but such nodes were never >>> + * exposed. Still, some user-space has logic to handle them. >>> + * >>> * Setting DRM_MINOR_ACCEL to 32 gives enough space for more drm minors to >>> * be implemented before we hit any future >>> */ >>> enum drm_minor_type { >>> DRM_MINOR_PRIMARY = 0, >>> - DRM_MINOR_CONTROL = 1, >> >> Maybe rather leave this line in and comment it with "// obsolete". >> Otherwise someone might accidentally use 1 for something. > > Yeah... That's why I added the comment. But maybe better to leave the entry > indeed, even if we risk someone accidentally using it. You could still out-comment the line as a whole. Simply having it there will warn potential users. Best regards Thomas > >> In any case >> >> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de> >> >> for the whole series. > > Thanks for the review!
diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h index 010239392adf..a23cc2f6163f 100644 --- a/include/drm/drm_file.h +++ b/include/drm/drm_file.h @@ -53,12 +53,14 @@ struct file; /* Note that the values of this enum are ABI (it determines * /dev/dri/renderD* numbers). * + * There used to be a DRM_MINOR_CONTROL = 1 entry, but such nodes were never + * exposed. Still, some user-space has logic to handle them. + * * Setting DRM_MINOR_ACCEL to 32 gives enough space for more drm minors to * be implemented before we hit any future */ enum drm_minor_type { DRM_MINOR_PRIMARY = 0, - DRM_MINOR_CONTROL = 1, DRM_MINOR_RENDER = 2, DRM_MINOR_ACCEL = 32, };
This entry should never be used by the kernel. Record the historical context in a comment. Signed-off-by: Simon Ser <contact@emersion.fr> Cc: Christian König <christian.koenig@amd.com> Cc: James Zhu <James.Zhu@amd.com> Cc: Marek Olšák <marek.olsak@amd.com> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> --- include/drm/drm_file.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)