| Message ID | 20180125133020.23845-1-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 2018-01-25 08:30 AM, Ville Syrjala wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > We use 32bit bitmasks to track planes/crtcs/encoders/connectors. > Naturally we can only do that if the index of those objects stays > below 32. Issue a warning whenever we exceed that limit, hopefully > prompting someone to fix the problem. > > For connectors the issue is a bit more complicated as they can > be created/destroyed at runtime due to MST. So the problem is no > longer a purely theoretical programmer error. As the connector > indexes are allocated via ida, we can simply limit the maximum > value the ida is allowed to hand out. The error handling is already > in place. > > v2: Return an error to the caller (Harry) > v3: Print a debug message so that we know what happened (Maarten) > > Cc: Harry Wentland <harry.wentland@amd.com> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Reviewed-by: Harry Wentland <harry.wentland@amd.com> Harry > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/drm_connector.c | 9 +++++++-- > drivers/gpu/drm/drm_crtc.c | 4 ++++ > drivers/gpu/drm/drm_encoder.c | 4 ++++ > drivers/gpu/drm/drm_plane.c | 4 ++++ > 4 files changed, 19 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c > index b85a7749709d..16b9c3810af2 100644 > --- a/drivers/gpu/drm/drm_connector.c > +++ b/drivers/gpu/drm/drm_connector.c > @@ -205,9 +205,14 @@ int drm_connector_init(struct drm_device *dev, > connector->dev = dev; > connector->funcs = funcs; > > - ret = ida_simple_get(&config->connector_ida, 0, 0, GFP_KERNEL); > - if (ret < 0) > + /* connector index is used with 32bit bitmasks */ > + ret = ida_simple_get(&config->connector_ida, 0, 32, GFP_KERNEL); > + if (ret < 0) { > + DRM_DEBUG_KMS("Failed to allocate %s connector index: %d\n", > + drm_connector_enum_list[connector_type].name, > + ret); > goto out_put; > + } > connector->index = ret; > ret = 0; > > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > index f0556e654116..5b4be382a1d7 100644 > --- a/drivers/gpu/drm/drm_crtc.c > +++ b/drivers/gpu/drm/drm_crtc.c > @@ -282,6 +282,10 @@ int drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc, > WARN_ON(primary && primary->type != DRM_PLANE_TYPE_PRIMARY); > WARN_ON(cursor && cursor->type != DRM_PLANE_TYPE_CURSOR); > > + /* crtc index is used with 32bit bitmasks */ > + if (WARN_ON(config->num_crtc >= 32)) > + return -EINVAL; > + > crtc->dev = dev; > crtc->funcs = funcs; > > diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c > index 59e0ebe733f8..273e1c59c54a 100644 > --- a/drivers/gpu/drm/drm_encoder.c > +++ b/drivers/gpu/drm/drm_encoder.c > @@ -110,6 +110,10 @@ int drm_encoder_init(struct drm_device *dev, > { > int ret; > > + /* encoder index is used with 32bit bitmasks */ > + if (WARN_ON(dev->mode_config.num_encoder >= 32)) > + return -EINVAL; > + > ret = drm_mode_object_add(dev, &encoder->base, DRM_MODE_OBJECT_ENCODER); > if (ret) > return ret; > diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c > index 2c90519576a3..22b54663b6e7 100644 > --- a/drivers/gpu/drm/drm_plane.c > +++ b/drivers/gpu/drm/drm_plane.c > @@ -173,6 +173,10 @@ int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane, > unsigned int format_modifier_count = 0; > int ret; > > + /* plane index is used with 32bit bitmasks */ > + if (WARN_ON(config->num_total_plane >= 32)) > + return -EINVAL; > + > ret = drm_mode_object_add(dev, &plane->base, DRM_MODE_OBJECT_PLANE); > if (ret) > return ret; >
On Thu, Jan 25, 2018 at 10:12:52AM -0500, Harry Wentland wrote: > On 2018-01-25 08:30 AM, Ville Syrjala wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > We use 32bit bitmasks to track planes/crtcs/encoders/connectors. > > Naturally we can only do that if the index of those objects stays > > below 32. Issue a warning whenever we exceed that limit, hopefully > > prompting someone to fix the problem. > > > > For connectors the issue is a bit more complicated as they can > > be created/destroyed at runtime due to MST. So the problem is no > > longer a purely theoretical programmer error. As the connector > > indexes are allocated via ida, we can simply limit the maximum > > value the ida is allowed to hand out. The error handling is already > > in place. > > > > v2: Return an error to the caller (Harry) > > v3: Print a debug message so that we know what happened (Maarten) > > > > Cc: Harry Wentland <harry.wentland@amd.com> > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > > Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > Reviewed-by: Harry Wentland <harry.wentland@amd.com> Thanks for the review. Pushed to drm-misc-next. > > Harry > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > --- > > drivers/gpu/drm/drm_connector.c | 9 +++++++-- > > drivers/gpu/drm/drm_crtc.c | 4 ++++ > > drivers/gpu/drm/drm_encoder.c | 4 ++++ > > drivers/gpu/drm/drm_plane.c | 4 ++++ > > 4 files changed, 19 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c > > index b85a7749709d..16b9c3810af2 100644 > > --- a/drivers/gpu/drm/drm_connector.c > > +++ b/drivers/gpu/drm/drm_connector.c > > @@ -205,9 +205,14 @@ int drm_connector_init(struct drm_device *dev, > > connector->dev = dev; > > connector->funcs = funcs; > > > > - ret = ida_simple_get(&config->connector_ida, 0, 0, GFP_KERNEL); > > - if (ret < 0) > > + /* connector index is used with 32bit bitmasks */ > > + ret = ida_simple_get(&config->connector_ida, 0, 32, GFP_KERNEL); > > + if (ret < 0) { > > + DRM_DEBUG_KMS("Failed to allocate %s connector index: %d\n", > > + drm_connector_enum_list[connector_type].name, > > + ret); > > goto out_put; > > + } > > connector->index = ret; > > ret = 0; > > > > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > > index f0556e654116..5b4be382a1d7 100644 > > --- a/drivers/gpu/drm/drm_crtc.c > > +++ b/drivers/gpu/drm/drm_crtc.c > > @@ -282,6 +282,10 @@ int drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc, > > WARN_ON(primary && primary->type != DRM_PLANE_TYPE_PRIMARY); > > WARN_ON(cursor && cursor->type != DRM_PLANE_TYPE_CURSOR); > > > > + /* crtc index is used with 32bit bitmasks */ > > + if (WARN_ON(config->num_crtc >= 32)) > > + return -EINVAL; > > + > > crtc->dev = dev; > > crtc->funcs = funcs; > > > > diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c > > index 59e0ebe733f8..273e1c59c54a 100644 > > --- a/drivers/gpu/drm/drm_encoder.c > > +++ b/drivers/gpu/drm/drm_encoder.c > > @@ -110,6 +110,10 @@ int drm_encoder_init(struct drm_device *dev, > > { > > int ret; > > > > + /* encoder index is used with 32bit bitmasks */ > > + if (WARN_ON(dev->mode_config.num_encoder >= 32)) > > + return -EINVAL; > > + > > ret = drm_mode_object_add(dev, &encoder->base, DRM_MODE_OBJECT_ENCODER); > > if (ret) > > return ret; > > diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c > > index 2c90519576a3..22b54663b6e7 100644 > > --- a/drivers/gpu/drm/drm_plane.c > > +++ b/drivers/gpu/drm/drm_plane.c > > @@ -173,6 +173,10 @@ int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane, > > unsigned int format_modifier_count = 0; > > int ret; > > > > + /* plane index is used with 32bit bitmasks */ > > + if (WARN_ON(config->num_total_plane >= 32)) > > + return -EINVAL; > > + > > ret = drm_mode_object_add(dev, &plane->base, DRM_MODE_OBJECT_PLANE); > > if (ret) > > return ret; > >
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index b85a7749709d..16b9c3810af2 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -205,9 +205,14 @@ int drm_connector_init(struct drm_device *dev, connector->dev = dev; connector->funcs = funcs; - ret = ida_simple_get(&config->connector_ida, 0, 0, GFP_KERNEL); - if (ret < 0) + /* connector index is used with 32bit bitmasks */ + ret = ida_simple_get(&config->connector_ida, 0, 32, GFP_KERNEL); + if (ret < 0) { + DRM_DEBUG_KMS("Failed to allocate %s connector index: %d\n", + drm_connector_enum_list[connector_type].name, + ret); goto out_put; + } connector->index = ret; ret = 0; diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index f0556e654116..5b4be382a1d7 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -282,6 +282,10 @@ int drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc, WARN_ON(primary && primary->type != DRM_PLANE_TYPE_PRIMARY); WARN_ON(cursor && cursor->type != DRM_PLANE_TYPE_CURSOR); + /* crtc index is used with 32bit bitmasks */ + if (WARN_ON(config->num_crtc >= 32)) + return -EINVAL; + crtc->dev = dev; crtc->funcs = funcs; diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c index 59e0ebe733f8..273e1c59c54a 100644 --- a/drivers/gpu/drm/drm_encoder.c +++ b/drivers/gpu/drm/drm_encoder.c @@ -110,6 +110,10 @@ int drm_encoder_init(struct drm_device *dev, { int ret; + /* encoder index is used with 32bit bitmasks */ + if (WARN_ON(dev->mode_config.num_encoder >= 32)) + return -EINVAL; + ret = drm_mode_object_add(dev, &encoder->base, DRM_MODE_OBJECT_ENCODER); if (ret) return ret; diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c index 2c90519576a3..22b54663b6e7 100644 --- a/drivers/gpu/drm/drm_plane.c +++ b/drivers/gpu/drm/drm_plane.c @@ -173,6 +173,10 @@ int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane, unsigned int format_modifier_count = 0; int ret; + /* plane index is used with 32bit bitmasks */ + if (WARN_ON(config->num_total_plane >= 32)) + return -EINVAL; + ret = drm_mode_object_add(dev, &plane->base, DRM_MODE_OBJECT_PLANE); if (ret) return ret;