[PATCH 0/4] drm/sysfs: per connector type attribute groups

[PATCH 0/4] drm/sysfs: per connector type attribute groups

[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(-)

-- 
2.1.4

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

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

[Jani Nikula]

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

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/drm_sysfs.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
index 5c99d3773212..9ce4d62feb46 100644
--- a/drivers/gpu/drm/drm_sysfs.c
+++ b/drivers/gpu/drm/drm_sysfs.c
@@ -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:
-- 
2.1.4

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

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

[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 <jani.nikula@intel.com>
---
 drivers/gpu/drm/drm_sysfs.c | 32 ++++++++++++++++++++------------
 1 file changed, 20 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
index 9ce4d62feb46..edb23c36c74a 100644
--- a/drivers/gpu/drm/drm_sysfs.c
+++ b/drivers/gpu/drm/drm_sysfs.c
@@ -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
 };
 
-- 
2.1.4

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

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

[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 <jani.nikula@intel.com>
---
 drivers/gpu/drm/drm_sysfs.c | 148 ++++++++++++++++++++++++++++++--------------
 1 file changed, 101 insertions(+), 47 deletions(-)

diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
index edb23c36c74a..d18c5aac6935 100644
--- a/drivers/gpu/drm/drm_sysfs.c
+++ b/drivers/gpu/drm/drm_sysfs.c
@@ -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,
 };
 
-- 
2.1.4

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

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

[Jani Nikula]

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

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/drm_sysfs.c | 32 --------------------------------
 1 file changed, 32 deletions(-)

diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
index d18c5aac6935..60b9f136ac44 100644
--- a/drivers/gpu/drm/drm_sysfs.c
+++ b/drivers/gpu/drm/drm_sysfs.c
@@ -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");
-- 
2.1.4

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

Re: [PATCH 0/4] drm/sysfs: per connector type attribute groups

[Jani Nikula]

Ping, anyone up for review?

BR,
Jani.

On Tue, 24 Feb 2015, Jani Nikula <jani.nikula@intel.com> wrote:
> 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(-)
>
> -- 
> 2.1.4
>

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