[PATCH] drm: support up to 128 drm devices

[Simon Ser]

(cc Daniel Vetter and Pekka because this change has uAPI repercussions)

On Friday, June 30th, 2023 at 13:56, James Zhu <James.Zhu at amd.com> wrote:

> From: Christian König <ckoenig.leichtzumerken at gmail.com>
> 
> This makes room for up to 128 DRM devices.
> 
> Signed-off-by: Christian König <christian.koenig at amd.com>
> Signed-off-by: James Zhu <James.Zhu at amd.com>
> ---
>  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..0d55c64444f5 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),

The type comes from this enum:

    enum drm_minor_type {
    	DRM_MINOR_PRIMARY,
    	DRM_MINOR_CONTROL,
    	DRM_MINOR_RENDER,
    	DRM_MINOR_ACCEL = 32,
    };

Before this patch, 0..63 are for primary, 64..127 are for control (never
exposed by the kernel), 128..191 are for render, 2048..2112 are for accel.
After this patch, 0..127 are for primary, 64..191 are for control (never
exposed by the kernel), 128..255 are for render, 2048..2176 are for accel.

I'm worried what might happen with old user-space, especially old libdrm.
When there are a lot of primary nodes, some would get detected as control
nodes, even if they aren't. Is this fine? This would at least be confusing
for information-gathering tools like drm_info. I'm not sure about other
consequences. Do we want to forever forbid the 64..127 range instead, so
that we have the guarantee that old libdrm never sees it?

I'm also worried about someone adding a new entry to the enum after
DRM_MINOR_RENDER (DRM_MINOR_ACCEL was specifically set to 32 so that new
node types could be added before). drm_minor_alloc() would blow up in this
case, because it'd use the 192..319 range, which overlaps with render.
I think a switch with explicit ranges (and WARN when an unknown type is
passed in) would be both easier to read and less risky.

>  			GFP_NOWAIT);
>  		spin_unlock_irqrestore(&drm_minor_lock, flags);
>  	}
> --
> 2.34.1