drm/sysfs: per connector type attribute groups

PATCH Superseded Jani Nikula 1 participant 6 messages Feb 24, 2015 – Mar 5, 2015

Subsystems (2)

DRM drivers DRM misc

lore

Share & tools

lore

Jani Nikula

author

2015-02-24 13:06 UTC #0 lore raw

cover drm/sysfs: per connector type attribute groups

Hi all, I was planning on adding some connector type specific stuff in the sysfs for DP, but ended up scrapping the plan midway through it. However I was left with some drm sysfs cleanup to make it easier; here are the *untested* patches. Basically the series addresses the code comment from Dave since the dawn of modesetting time:

	/*
	 * In the long run it maybe a good idea to make one set of
	 * optionals per connector type.
	 */

Any functional or user visible changes in the series are bugs.

BR,
Jani.

Jani Nikula (4):
drm/sysfs: add a helper for extracting connector type from kobject drm/sysfs: make optional attribute groups per connector type drm/sysfs: split DVI-I and TV-out attributes
drm/sysfs: remove unnecessary connector type checks

 drivers/gpu/drm/drm_sysfs.c | 160 +++++++++++++++++++++++++++-----------------
 1 file changed, 98 insertions(+), 62 deletions(-)

Signature · 6 lines

2.1.4

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Jani Nikula

author

2015-02-24 13:06 UTC #1 lore raw

PATCH 1/4 drm/sysfs: add a helper for extracting connector type from kobject

This reduces duplication in the patches to follow. No functional changes.

Signed-off-by Jani Nikula <[email protected]>

---
 drivers/gpu/drm/drm_sysfs.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

drivers/gpu/drm/drm_sysfs.c +9 -3

index 5c99d3773212..9ce4d62feb46 100644
@@ -362,17 +362,23 @@ static struct attribute *connector_opt_dev_attrs[] = {
 	NULL
 };
 
-static umode_t connector_opt_dev_is_visible(struct kobject *kobj,
-					    struct attribute *attr, int idx)
+/* Connector type related helpers */
+static int kobj_connector_type(struct kobject *kobj)
 {
 	struct device *dev = kobj_to_dev(kobj);
 	struct drm_connector *connector = to_drm_connector(dev);
 
+	return connector->connector_type;
+}
+
+static umode_t connector_opt_dev_is_visible(struct kobject *kobj,
+					    struct attribute *attr, int idx)
+{
 	/*
 	 * In the long run it maybe a good idea to make one set of
 	 * optionals per connector type.
 	 */
-	switch (connector->connector_type) {
+	switch (kobj_connector_type(kobj)) {
 	case DRM_MODE_CONNECTOR_DVII:
 	case DRM_MODE_CONNECTOR_Composite:
 	case DRM_MODE_CONNECTOR_SVIDEO:

Signature · 6 lines

2.1.4

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Jani Nikula

author

2015-02-24 13:06 UTC #2 lore raw

PATCH 2/4 drm/sysfs: make optional attribute groups per connector type

In reply to Jani Nikula

Split DVI-I and TV-out (which remains a group of types). As an intermediate step, still share the attributes themselves between the two. No user visible changes.

Signed-off-by Jani Nikula <[email protected]>

---
 drivers/gpu/drm/drm_sysfs.c | 32 ++++++++++++++++++++------------
 1 file changed, 20 insertions(+), 12 deletions(-)

drivers/gpu/drm/drm_sysfs.c +20 ~~-12~~

index 9ce4d62feb46..edb23c36c74a 100644
@@ -356,7 +356,7 @@ static struct attribute *connector_dev_attrs[] = {
 static DEVICE_ATTR_RO(subconnector);
 static DEVICE_ATTR_RO(select_subconnector);
 
-static struct attribute *connector_opt_dev_attrs[] = {
+static struct attribute *connector_tv_dev_attrs[] = {
 	&dev_attr_subconnector.attr,
 	&dev_attr_select_subconnector.attr,
 	NULL

@@ -371,15 +371,17 @@ static int kobj_connector_type(struct kobject *kobj)
 	return connector->connector_type;
 }
 
-static umode_t connector_opt_dev_is_visible(struct kobject *kobj,
-					    struct attribute *attr, int idx)
+static umode_t connector_is_dvii(struct kobject *kobj,
+				 struct attribute *attr, int idx)
+{
+	return kobj_connector_type(kobj) == DRM_MODE_CONNECTOR_DVII ?
+		attr->mode : 0;
+}
+
+static umode_t connector_is_tv(struct kobject *kobj,
+			       struct attribute *attr, int idx)
 {
-	/*
-	 * In the long run it maybe a good idea to make one set of
-	 * optionals per connector type.
-	 */
 	switch (kobj_connector_type(kobj)) {
-	case DRM_MODE_CONNECTOR_DVII:
 	case DRM_MODE_CONNECTOR_Composite:
 	case DRM_MODE_CONNECTOR_SVIDEO:
 	case DRM_MODE_CONNECTOR_Component:

@@ -407,14 +409,20 @@ static const struct attribute_group connector_dev_group = {
 	.bin_attrs = connector_bin_attrs,
 };
 
-static const struct attribute_group connector_opt_dev_group = {
-	.attrs = connector_opt_dev_attrs,
-	.is_visible = connector_opt_dev_is_visible,
+static const struct attribute_group connector_tv_dev_group = {
+	.attrs = connector_tv_dev_attrs,
+	.is_visible = connector_is_tv,
+};
+
+static const struct attribute_group connector_dvii_dev_group = {
+	.attrs = connector_tv_dev_attrs, /* same as tv */
+	.is_visible = connector_is_dvii,
 };
 
 static const struct attribute_group *connector_dev_groups[] = {
 	&connector_dev_group,
-	&connector_opt_dev_group,
+	&connector_tv_dev_group,
+	&connector_dvii_dev_group,
 	NULL
 };

Signature · 6 lines

2.1.4

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Jani Nikula

author

2015-02-24 13:06 UTC #3 lore raw

PATCH 3/4 drm/sysfs: split DVI-I and TV-out attributes

In reply to Jani Nikula

The show methods for the attributes of DVI-I and TV-out types have a bunch of code to deal with the differences between the two. Just split the attributes into connector type specific ones. No functional changes.

Signed-off-by Jani Nikula <[email protected]>

---
 drivers/gpu/drm/drm_sysfs.c | 148 ++++++++++++++++++++++++++++++--------------
 1 file changed, 101 insertions(+), 47 deletions(-)

drivers/gpu/drm/drm_sysfs.c +101 ~~-47~~

index edb23c36c74a..d18c5aac6935 100644
@@ -257,33 +257,28 @@ static ssize_t modes_show(struct device *device,
 	return written;
 }
 
-static ssize_t subconnector_show(struct device *device,
-			   struct device_attribute *attr,
-			   char *buf)
+static ssize_t tv_subconnector_show(struct device *device,
+				    struct device_attribute *attr,
+				    char *buf)
 {
 	struct drm_connector *connector = to_drm_connector(device);
 	struct drm_device *dev = connector->dev;
-	struct drm_property *prop = NULL;
+	struct drm_property *prop;
 	uint64_t subconnector;
-	int is_tv = 0;
 	int ret;
 
 	switch (connector->connector_type) {
-		case DRM_MODE_CONNECTOR_DVII:
-			prop = dev->mode_config.dvi_i_subconnector_property;
-			break;
-		case DRM_MODE_CONNECTOR_Composite:
-		case DRM_MODE_CONNECTOR_SVIDEO:
-		case DRM_MODE_CONNECTOR_Component:
-		case DRM_MODE_CONNECTOR_TV:
-			prop = dev->mode_config.tv_subconnector_property;
-			is_tv = 1;
-			break;
-		default:
-			DRM_ERROR("Wrong connector type for this property\n");
-			return 0;
+	case DRM_MODE_CONNECTOR_Composite:
+	case DRM_MODE_CONNECTOR_SVIDEO:
+	case DRM_MODE_CONNECTOR_Component:
+	case DRM_MODE_CONNECTOR_TV:
+		break;
+	default:
+		DRM_ERROR("Wrong connector type for this property\n");
+		return 0;
 	}
 
+	prop = dev->mode_config.tv_subconnector_property;
 	if (!prop) {
 		DRM_ERROR("Unable to find subconnector property\n");
 		return 0;

@@ -293,38 +288,90 @@ static ssize_t subconnector_show(struct device *device,
 	if (ret)
 		return 0;
 
-	return snprintf(buf, PAGE_SIZE, "%s", is_tv ?
-			drm_get_tv_subconnector_name((int)subconnector) :
-			drm_get_dvi_i_subconnector_name((int)subconnector));
+	return snprintf(buf, PAGE_SIZE, "%s",
+			drm_get_tv_subconnector_name((int)subconnector));
 }
 
-static ssize_t select_subconnector_show(struct device *device,
-			   struct device_attribute *attr,
-			   char *buf)
+static ssize_t tv_select_subconnector_show(struct device *device,
+					   struct device_attribute *attr,
+					   char *buf)
 {
 	struct drm_connector *connector = to_drm_connector(device);
 	struct drm_device *dev = connector->dev;
-	struct drm_property *prop = NULL;
+	struct drm_property *prop;
 	uint64_t subconnector;
-	int is_tv = 0;
 	int ret;
 
 	switch (connector->connector_type) {
-		case DRM_MODE_CONNECTOR_DVII:
-			prop = dev->mode_config.dvi_i_select_subconnector_property;
-			break;
-		case DRM_MODE_CONNECTOR_Composite:
-		case DRM_MODE_CONNECTOR_SVIDEO:
-		case DRM_MODE_CONNECTOR_Component:
-		case DRM_MODE_CONNECTOR_TV:
-			prop = dev->mode_config.tv_select_subconnector_property;
-			is_tv = 1;
-			break;
-		default:
-			DRM_ERROR("Wrong connector type for this property\n");
-			return 0;
+	case DRM_MODE_CONNECTOR_Composite:
+	case DRM_MODE_CONNECTOR_SVIDEO:
+	case DRM_MODE_CONNECTOR_Component:
+	case DRM_MODE_CONNECTOR_TV:
+		break;
+	default:
+		DRM_ERROR("Wrong connector type for this property\n");
+		return 0;
+	}
+
+	prop = dev->mode_config.tv_select_subconnector_property;
+	if (!prop) {
+		DRM_ERROR("Unable to find select subconnector property\n");
+		return 0;
+	}
+
+	ret = drm_object_property_get_value(&connector->base, prop, &subconnector);
+	if (ret)
+		return 0;
+
+	return snprintf(buf, PAGE_SIZE, "%s",
+			drm_get_tv_select_name((int)subconnector));
+}
+
+static ssize_t dvii_subconnector_show(struct device *device,
+				      struct device_attribute *attr,
+				      char *buf)
+{
+	struct drm_connector *connector = to_drm_connector(device);
+	struct drm_device *dev = connector->dev;
+	struct drm_property *prop;
+	uint64_t subconnector;
+	int ret;
+
+	if (connector->connector_type != DRM_MODE_CONNECTOR_DVII) {
+		DRM_ERROR("Wrong connector type for this property\n");
+		return 0;
+	}
+
+	prop = dev->mode_config.dvi_i_subconnector_property;
+	if (!prop) {
+		DRM_ERROR("Unable to find subconnector property\n");
+		return 0;
+	}
+
+	ret = drm_object_property_get_value(&connector->base, prop, &subconnector);
+	if (ret)
+		return 0;
+
+	return snprintf(buf, PAGE_SIZE, "%s",
+			drm_get_dvi_i_subconnector_name((int)subconnector));
+}
+
+static ssize_t dvii_select_subconnector_show(struct device *device,
+					     struct device_attribute *attr,
+					     char *buf)
+{
+	struct drm_connector *connector = to_drm_connector(device);
+	struct drm_device *dev = connector->dev;
+	struct drm_property *prop;
+	uint64_t subconnector;
+	int ret;
+
+	if (connector->connector_type != DRM_MODE_CONNECTOR_DVII) {
+		DRM_ERROR("Wrong connector type for this property\n");
+		return 0;
 	}
 
+	prop = dev->mode_config.dvi_i_select_subconnector_property;
 	if (!prop) {
 		DRM_ERROR("Unable to find select subconnector property\n");
 		return 0;

@@ -334,8 +381,7 @@ static ssize_t select_subconnector_show(struct device *device,
 	if (ret)
 		return 0;
 
-	return snprintf(buf, PAGE_SIZE, "%s", is_tv ?
-			drm_get_tv_select_name((int)subconnector) :
+	return snprintf(buf, PAGE_SIZE, "%s",
 			drm_get_dvi_i_select_name((int)subconnector));
 }

@@ -352,13 +398,21 @@ static struct attribute *connector_dev_attrs[] = {
 	NULL
 };
 
-/* These attributes are for both DVI-I connectors and all types of tv-out. */
-static DEVICE_ATTR_RO(subconnector);
-static DEVICE_ATTR_RO(select_subconnector);
+static DEVICE_ATTR_RO(tv_subconnector);
+static DEVICE_ATTR_RO(tv_select_subconnector);
 
 static struct attribute *connector_tv_dev_attrs[] = {
-	&dev_attr_subconnector.attr,
-	&dev_attr_select_subconnector.attr,
+	&dev_attr_tv_subconnector.attr,
+	&dev_attr_tv_select_subconnector.attr,
+	NULL
+};
+
+static DEVICE_ATTR_RO(dvii_subconnector);
+static DEVICE_ATTR_RO(dvii_select_subconnector);
+
+static struct attribute *connector_dvii_dev_attrs[] = {
+	&dev_attr_dvii_subconnector.attr,
+	&dev_attr_dvii_select_subconnector.attr,
 	NULL
 };

@@ -415,7 +469,7 @@ static const struct attribute_group connector_tv_dev_group = {
 };
 
 static const struct attribute_group connector_dvii_dev_group = {
-	.attrs = connector_tv_dev_attrs, /* same as tv */
+	.attrs = connector_dvii_dev_attrs,
 	.is_visible = connector_is_dvii,
 };

Signature · 6 lines

2.1.4

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Jani Nikula

author

2015-02-24 13:07 UTC #4 lore raw

PATCH 4/4 drm/sysfs: remove unnecessary connector type checks

In reply to Jani Nikula

These attributes should be exposed for the matching connector types only, so checking is redundant.

Signed-off-by Jani Nikula <[email protected]>

---
 drivers/gpu/drm/drm_sysfs.c | 32 --------------------------------
 1 file changed, 32 deletions(-)

drivers/gpu/drm/drm_sysfs.c +0 ~~-32~~

index d18c5aac6935..60b9f136ac44 100644
@@ -267,17 +267,6 @@ static ssize_t tv_subconnector_show(struct device *device,
 	uint64_t subconnector;
 	int ret;
 
-	switch (connector->connector_type) {
-	case DRM_MODE_CONNECTOR_Composite:
-	case DRM_MODE_CONNECTOR_SVIDEO:
-	case DRM_MODE_CONNECTOR_Component:
-	case DRM_MODE_CONNECTOR_TV:
-		break;
-	default:
-		DRM_ERROR("Wrong connector type for this property\n");
-		return 0;
-	}
-
 	prop = dev->mode_config.tv_subconnector_property;
 	if (!prop) {
 		DRM_ERROR("Unable to find subconnector property\n");

@@ -302,17 +291,6 @@ static ssize_t tv_select_subconnector_show(struct device *device,
 	uint64_t subconnector;
 	int ret;
 
-	switch (connector->connector_type) {
-	case DRM_MODE_CONNECTOR_Composite:
-	case DRM_MODE_CONNECTOR_SVIDEO:
-	case DRM_MODE_CONNECTOR_Component:
-	case DRM_MODE_CONNECTOR_TV:
-		break;
-	default:
-		DRM_ERROR("Wrong connector type for this property\n");
-		return 0;
-	}
-
 	prop = dev->mode_config.tv_select_subconnector_property;
 	if (!prop) {
 		DRM_ERROR("Unable to find select subconnector property\n");

@@ -337,11 +315,6 @@ static ssize_t dvii_subconnector_show(struct device *device,
 	uint64_t subconnector;
 	int ret;
 
-	if (connector->connector_type != DRM_MODE_CONNECTOR_DVII) {
-		DRM_ERROR("Wrong connector type for this property\n");
-		return 0;
-	}
-
 	prop = dev->mode_config.dvi_i_subconnector_property;
 	if (!prop) {
 		DRM_ERROR("Unable to find subconnector property\n");

@@ -366,11 +339,6 @@ static ssize_t dvii_select_subconnector_show(struct device *device,
 	uint64_t subconnector;
 	int ret;
 
-	if (connector->connector_type != DRM_MODE_CONNECTOR_DVII) {
-		DRM_ERROR("Wrong connector type for this property\n");
-		return 0;
-	}
-
 	prop = dev->mode_config.dvi_i_select_subconnector_property;
 	if (!prop) {
 		DRM_ERROR("Unable to find select subconnector property\n");

Signature · 6 lines

2.1.4

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Jani Nikula

author

2015-03-05 15:26 UTC #5 lore raw

In reply to Jani Nikula

Ping, anyone up for review?

BR,
Jani.

Quoted 29 lines · Jani Nikula

Hi all, I was planning on adding some connector type specific stuff in the sysfs for DP, but ended up scrapping the plan midway through it. However I was left with some drm sysfs cleanup to make it easier; here are the *untested* patches. Basically the series addresses the code comment from Dave since the dawn of modesetting time:
	/*
	 * In the long run it maybe a good idea to make one set of
	 * optionals per connector type.
	 */
Any functional or user visible changes in the series are bugs.
BR,
Jani.
Jani Nikula (4):
drm/sysfs: add a helper for extracting connector type from kobject drm/sysfs: make optional attribute groups per connector type drm/sysfs: split DVI-I and TV-out attributes
drm/sysfs: remove unnecessary connector type checks
 drivers/gpu/drm/drm_sysfs.c | 160 +++++++++++++++++++++++++++-----------------
 1 file changed, 98 insertions(+), 62 deletions(-)
Signature · 1 line
2.1.4

Signature · 5 lines

Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel