drm: s/enum_blob_list/enum_list/ in drm_property

I guess for hysterical raisins this was meant to be the way to read
blob properties. But that's done with the two-stage approach which
uses separate blob kms object and the special-purpose get_blob ioctl.

Shipping userspace seems to have never relied on this, and the kernel
also never put any blob thing onto that property. And nowadays it
would blow up, e.g. in drm_property_destroy. Also it makes no sense to
return values in an ioctl that only returns metadata about everything.

So let's ditch all the internal code for the blob list, rename the
list to be unambiguous and sprinkle comments all over the place to
explain this peculiar piece of api.

v2: Squash in fixup from Rob to remove now unused variables.

Cc: Rob Clark <robdclark@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Reviewed-by: Rob Clark <robdclark@gmail.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 8c550302a9eff293beb62b60cb3ca0fe4314dd9e..589a921d4313295df390142396bf12cc058b90f0 100644 (file)
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -3457,7 +3457,7 @@ struct drm_property *drm_property_create(struct drm_device *dev, int flags,
 
        property->flags = flags;
        property->num_values = num_values;
-       INIT_LIST_HEAD(&property->enum_blob_list);
+       INIT_LIST_HEAD(&property->enum_list);
 
        if (name) {
                strncpy(property->name, name, DRM_PROP_NAME_LEN);
@@ -3679,8 +3679,8 @@ int drm_property_add_enum(struct drm_property *property, int index,
                        (value > 63))
                return -EINVAL;
 
-       if (!list_empty(&property->enum_blob_list)) {
-               list_for_each_entry(prop_enum, &property->enum_blob_list, head) {
+       if (!list_empty(&property->enum_list)) {
+               list_for_each_entry(prop_enum, &property->enum_list, head) {
                        if (prop_enum->value == value) {
                                strncpy(prop_enum->name, name, DRM_PROP_NAME_LEN);
                                prop_enum->name[DRM_PROP_NAME_LEN-1] = '\0';
@@ -3698,7 +3698,7 @@ int drm_property_add_enum(struct drm_property *property, int index,
        prop_enum->value = value;
 
        property->values[index] = value;
-       list_add_tail(&prop_enum->head, &property->enum_blob_list);
+       list_add_tail(&prop_enum->head, &property->enum_list);
        return 0;
 }
 EXPORT_SYMBOL(drm_property_add_enum);
@@ -3715,7 +3715,7 @@ void drm_property_destroy(struct drm_device *dev, struct drm_property *property)
 {
        struct drm_property_enum *prop_enum, *pt;
 
-       list_for_each_entry_safe(prop_enum, pt, &property->enum_blob_list, head) {
+       list_for_each_entry_safe(prop_enum, pt, &property->enum_list, head) {
                list_del(&prop_enum->head);
                kfree(prop_enum);
        }
@@ -3839,16 +3839,12 @@ int drm_mode_getproperty_ioctl(struct drm_device *dev,
        struct drm_mode_get_property *out_resp = data;
        struct drm_property *property;
        int enum_count = 0;
-       int blob_count = 0;
        int value_count = 0;
        int ret = 0, i;
        int copied;
        struct drm_property_enum *prop_enum;
        struct drm_mode_property_enum __user *enum_ptr;
-       struct drm_property_blob *prop_blob;
-       uint32_t __user *blob_id_ptr;
        uint64_t __user *values_ptr;
-       uint32_t __user *blob_length_ptr;
 
        if (!drm_core_check_feature(dev, DRIVER_MODESET))
                return -EINVAL;
@@ -3862,11 +3858,8 @@ int drm_mode_getproperty_ioctl(struct drm_device *dev,
 
        if (drm_property_type_is(property, DRM_MODE_PROP_ENUM) ||
                        drm_property_type_is(property, DRM_MODE_PROP_BITMASK)) {
-               list_for_each_entry(prop_enum, &property->enum_blob_list, head)
+               list_for_each_entry(prop_enum, &property->enum_list, head)
                        enum_count++;
-       } else if (drm_property_type_is(property, DRM_MODE_PROP_BLOB)) {
-               list_for_each_entry(prop_blob, &property->enum_blob_list, head)
-                       blob_count++;
        }
 
        value_count = property->num_values;
@@ -3891,7 +3884,7 @@ int drm_mode_getproperty_ioctl(struct drm_device *dev,
                if ((out_resp->count_enum_blobs >= enum_count) && enum_count) {
                        copied = 0;
                        enum_ptr = (struct drm_mode_property_enum __user *)(unsigned long)out_resp->enum_blob_ptr;
-                       list_for_each_entry(prop_enum, &property->enum_blob_list, head) {
+                       list_for_each_entry(prop_enum, &property->enum_list, head) {
 
                                if (copy_to_user(&enum_ptr[copied].value, &prop_enum->value, sizeof(uint64_t))) {
                                        ret = -EFAULT;
@@ -3909,28 +3902,16 @@ int drm_mode_getproperty_ioctl(struct drm_device *dev,
                out_resp->count_enum_blobs = enum_count;
        }
 
-       if (drm_property_type_is(property, DRM_MODE_PROP_BLOB)) {
-               if ((out_resp->count_enum_blobs >= blob_count) && blob_count) {
-                       copied = 0;
-                       blob_id_ptr = (uint32_t __user *)(unsigned long)out_resp->enum_blob_ptr;
-                       blob_length_ptr = (uint32_t __user *)(unsigned long)out_resp->values_ptr;
-
-                       list_for_each_entry(prop_blob, &property->enum_blob_list, head) {
-                               if (put_user(prop_blob->base.id, blob_id_ptr + copied)) {
-                                       ret = -EFAULT;
-                                       goto done;
-                               }
-
-                               if (put_user(prop_blob->length, blob_length_ptr + copied)) {
-                                       ret = -EFAULT;
-                                       goto done;
-                               }
-
-                               copied++;
-                       }
-               }
-               out_resp->count_enum_blobs = blob_count;
-       }
+       /*
+        * NOTE: The idea seems to have been to use this to read all the blob
+        * property values. But nothing ever added them to the corresponding
+        * list, userspace always used the special-purpose get_blob ioctl to
+        * read the value for a blob property. It also doesn't make a lot of
+        * sense to return values here when everything else is just metadata for
+        * the property itself.
+        */
+       if (drm_property_type_is(property, DRM_MODE_PROP_BLOB))
+               out_resp->count_enum_blobs = 0;
 done:
        drm_modeset_unlock_all(dev);
        return ret;

diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index f3142c64e3d54b2958cfec7645598a66bbd9738d..b459e8fbbc25f1d3d63d60dda523b0cde6159f9e 100644 (file)
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -216,7 +216,7 @@ struct drm_property {
        uint64_t *values;
        struct drm_device *dev;
 
-       struct list_head enum_blob_list;
+       struct list_head enum_list;
 };
 
 struct drm_crtc;

diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index a0db2d4aa5f0125349adc7ec6516fbfbda63be43..86574b0005ff0a476fa2299b8ef73de418670024 100644 (file)
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -286,6 +286,8 @@ struct drm_mode_get_property {
        char name[DRM_PROP_NAME_LEN];
 
        __u32 count_values;
+       /* This is only used to count enum values, not blobs. The _blobs is
+        * simply because of a historical reason, i.e. backwards compat. */
        __u32 count_enum_blobs;
 };