Re: [RFC PATCH 1/2] drm: store connector name in connector struct

Hi

On Wed, May 14, 2014 at 3:58 PM, Jani Nikula jani.nikula@intel.com wrote:

This makes drm_get_connector_name() thread safe.

Reference: http://lkml.kernel.org/r/645ee6e22cad47d38a2b35c21c8d5fe3@DC1-MBX-01.ptsecur... Signed-off-by: Jani Nikula jani.nikula@intel.com

I like that approach, but we should also kill drm_get_connector_name() in a follow-up. I don't see why that wrapper is needed.

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

Thanks David

---

drivers/gpu/drm/drm_crtc.c | 36 ++++++++++++++++++++---------------- include/drm/drm_crtc.h | 2 ++ 2 files changed, 22 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 461d19bd14ee..5781130b4126 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -277,21 +277,11 @@ EXPORT_SYMBOL(drm_get_encoder_name);

/**

• drm_get_connector_name - return a string for connector

• • @connector: connector 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.

• • @connector: the connector to get name for
  */

const char *drm_get_connector_name(const struct drm_connector *connector) {

•   static char buf[32];
  

•   snprintf(buf, 32, "%s-%d",
  

•            drm_connector_enum_list[connector->connector_type].name,
  

•            connector->connector_type_id);
  

•   return buf;
  

•   return connector->name;
  

} EXPORT_SYMBOL(drm_get_connector_name);

@@ -824,7 +814,7 @@ int drm_connector_init(struct drm_device *dev,

    ret = drm_mode_object_get(dev, &connector->base, DRM_MODE_OBJECT_CONNECTOR);
    if (ret)

•           goto out;
  

•           goto out_unlock;
  
    connector->base.properties = &connector->properties;
    connector->dev = dev;
  

@@ -834,9 +824,17 @@ int drm_connector_init(struct drm_device *dev, ida_simple_get(connector_ida, 1, 0, GFP_KERNEL); if (connector->connector_type_id < 0) { ret = connector->connector_type_id;

•           drm_mode_object_put(dev, &connector->base);
  

•           goto out;
  

•           goto out_out;
    }
  

•   connector->name =
  

•           kasprintf(GFP_KERNEL, "%s-%d",
  

•                     drm_connector_enum_list[connector_type].name,
  

•                     connector->connector_type_id);
  

•   if (!connector->name) {
  

•           ret = -ENOMEM;
  

•           goto out_put;
  

•   }
  

•   INIT_LIST_HEAD(&connector->probed_modes);
    INIT_LIST_HEAD(&connector->modes);
    connector->edid_blob_ptr = NULL;
  

@@ -853,7 +851,11 @@ int drm_connector_init(struct drm_device *dev, drm_object_attach_property(&connector->base, dev->mode_config.dpms_property, 0);

• out:

+out_put:

•   if (ret)
  

•           drm_mode_object_put(dev, &connector->base);
  

+out_unlock: drm_modeset_unlock_all(dev);

    return ret;

@@ -881,6 +883,8 @@ void drm_connector_cleanup(struct drm_connector *connector) connector->connector_type_id);

    drm_mode_object_put(dev, &connector->base);

•   kfree(connector->name);
  

•   connector->name = NULL;
    list_del(&connector->head);
    dev->mode_config.num_connector--;
  

} diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index c061bb372199..d4cd7e513280 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -444,6 +444,7 @@ struct drm_encoder {

• @attr: sysfs attributes
• @head: list management
• @base: base KMS object

• • @name: connector name
  • @connector_type: one of the %DRM_MODE_CONNECTOR_<foo> types from drm_mode.h
  • @connector_type_id: index into connector type enum
  • @interlace_allowed: can this connector handle interlaced modes?

@@ -482,6 +483,7 @@ struct drm_connector {

    struct drm_mode_object base;

•   char *name;
    int connector_type;
    int connector_type_id;
    bool interlace_allowed;
  

-- 1.9.1

---

dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel