diff mbox

[RFC,2/2] drm: store encoder name in encoder struct

Message ID 8b3c3851a20e3f818145f79a43e00ad28dbeec28.1400075481.git.jani.nikula@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jani Nikula May 14, 2014, 1:58 p.m. UTC 
This makes drm_get_encoder_name() thread safe.

Reference: http://lkml.kernel.org/r/645ee6e22cad47d38a2b35c21c8d5fe3@DC1-MBX-01\
.ptsecurity.ru
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/drm_crtc.c | 31 +++++++++++++++++--------------
 include/drm/drm_crtc.h     |  2 ++
 2 files changed, 19 insertions(+), 14 deletions(-)

Comments

David Herrmann May 19, 2014, 1:08 p.m. UTC | #1 
Hi

On Wed, May 14, 2014 at 3:58 PM, Jani Nikula <jani.nikula@intel.com> wrote:
> This makes drm_get_encoder_name() thread safe.
>
> Reference: http://lkml.kernel.org/r/645ee6e22cad47d38a2b35c21c8d5fe3@DC1-MBX-01\
> .ptsecurity.ru
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

Same as for 1/2, a followup would be nice.

Reviewed-by: David Herrmann <dh.herrmann@gmail.com>

Thanks
David

> ---
>  drivers/gpu/drm/drm_crtc.c | 31 +++++++++++++++++--------------
>  include/drm/drm_crtc.h     |  2 ++
>  2 files changed, 19 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 5781130b4126..1c4cb74ede77 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -257,21 +257,11 @@ void drm_connector_ida_destroy(void)
>
>  /**
>   * drm_get_encoder_name - return a string for encoder
> - * @encoder: encoder to compute name of
> - *
> - * Note that the buffer used by this function is globally shared and owned by
> - * the function itself.
> - *
> - * FIXME: This isn't really multithreading safe.
> + * @encoder: the encoder to get name for
>   */
>  const char *drm_get_encoder_name(const struct drm_encoder *encoder)
>  {
> -       static char buf[32];
> -
> -       snprintf(buf, 32, "%s-%d",
> -                drm_encoder_enum_list[encoder->encoder_type].name,
> -                encoder->base.id);
> -       return buf;
> +       return encoder->name;
>  }
>  EXPORT_SYMBOL(drm_get_encoder_name);
>
> @@ -986,16 +976,27 @@ int drm_encoder_init(struct drm_device *dev,
>
>         ret = drm_mode_object_get(dev, &encoder->base, DRM_MODE_OBJECT_ENCODER);
>         if (ret)
> -               goto out;
> +               goto out_unlock;
>
>         encoder->dev = dev;
>         encoder->encoder_type = encoder_type;
>         encoder->funcs = funcs;
> +       encoder->name = kasprintf(GFP_KERNEL, "%s-%d",
> +                                 drm_encoder_enum_list[encoder_type].name,
> +                                 encoder->base.id);
> +       if (!encoder->name) {
> +               ret = -ENOMEM;
> +               goto out_put;
> +       }
>
>         list_add_tail(&encoder->head, &dev->mode_config.encoder_list);
>         dev->mode_config.num_encoder++;
>
> - out:
> +out_put:
> +       if (ret)
> +               drm_mode_object_put(dev, &encoder->base);
> +
> +out_unlock:
>         drm_modeset_unlock_all(dev);
>
>         return ret;
> @@ -1013,6 +1014,8 @@ void drm_encoder_cleanup(struct drm_encoder *encoder)
>         struct drm_device *dev = encoder->dev;
>         drm_modeset_lock_all(dev);
>         drm_mode_object_put(dev, &encoder->base);
> +       kfree(encoder->name);
> +       encoder->name = NULL;
>         list_del(&encoder->head);
>         dev->mode_config.num_encoder--;
>         drm_modeset_unlock_all(dev);
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index d4cd7e513280..219b533a2c15 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -400,6 +400,7 @@ struct drm_encoder_funcs {
>   * @dev: parent DRM device
>   * @head: list management
>   * @base: base KMS object
> + * @name: encoder name
>   * @encoder_type: one of the %DRM_MODE_ENCODER_<foo> types in drm_mode.h
>   * @possible_crtcs: bitmask of potential CRTC bindings
>   * @possible_clones: bitmask of potential sibling encoders for cloning
> @@ -416,6 +417,7 @@ struct drm_encoder {
>         struct list_head head;
>
>         struct drm_mode_object base;
> +       char *name;
>         int encoder_type;
>         uint32_t possible_crtcs;
>         uint32_t possible_clones;
> --
> 1.9.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
diff mbox

Patch

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 5781130b4126..1c4cb74ede77 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -257,21 +257,11 @@  void drm_connector_ida_destroy(void)
 
 /**
  * drm_get_encoder_name - return a string for encoder
- * @encoder: encoder to compute name of
- *
- * Note that the buffer used by this function is globally shared and owned by
- * the function itself.
- *
- * FIXME: This isn't really multithreading safe.
+ * @encoder: the encoder to get name for
  */
 const char *drm_get_encoder_name(const struct drm_encoder *encoder)
 {
-	static char buf[32];
-
-	snprintf(buf, 32, "%s-%d",
-		 drm_encoder_enum_list[encoder->encoder_type].name,
-		 encoder->base.id);
-	return buf;
+	return encoder->name;
 }
 EXPORT_SYMBOL(drm_get_encoder_name);
 
@@ -986,16 +976,27 @@  int drm_encoder_init(struct drm_device *dev,
 
 	ret = drm_mode_object_get(dev, &encoder->base, DRM_MODE_OBJECT_ENCODER);
 	if (ret)
-		goto out;
+		goto out_unlock;
 
 	encoder->dev = dev;
 	encoder->encoder_type = encoder_type;
 	encoder->funcs = funcs;
+	encoder->name = kasprintf(GFP_KERNEL, "%s-%d",
+				  drm_encoder_enum_list[encoder_type].name,
+				  encoder->base.id);
+	if (!encoder->name) {
+		ret = -ENOMEM;
+		goto out_put;
+	}
 
 	list_add_tail(&encoder->head, &dev->mode_config.encoder_list);
 	dev->mode_config.num_encoder++;
 
- out:
+out_put:
+	if (ret)
+		drm_mode_object_put(dev, &encoder->base);
+
+out_unlock:
 	drm_modeset_unlock_all(dev);
 
 	return ret;
@@ -1013,6 +1014,8 @@  void drm_encoder_cleanup(struct drm_encoder *encoder)
 	struct drm_device *dev = encoder->dev;
 	drm_modeset_lock_all(dev);
 	drm_mode_object_put(dev, &encoder->base);
+	kfree(encoder->name);
+	encoder->name = NULL;
 	list_del(&encoder->head);
 	dev->mode_config.num_encoder--;
 	drm_modeset_unlock_all(dev);
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index d4cd7e513280..219b533a2c15 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -400,6 +400,7 @@  struct drm_encoder_funcs {
  * @dev: parent DRM device
  * @head: list management
  * @base: base KMS object
+ * @name: encoder name
  * @encoder_type: one of the %DRM_MODE_ENCODER_<foo> types in drm_mode.h
  * @possible_crtcs: bitmask of potential CRTC bindings
  * @possible_clones: bitmask of potential sibling encoders for cloning
@@ -416,6 +417,7 @@  struct drm_encoder {
 	struct list_head head;
 
 	struct drm_mode_object base;
+	char *name;
 	int encoder_type;
 	uint32_t possible_crtcs;
 	uint32_t possible_clones;