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

[PATCH RESEND 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 ffc305fc2076..33466999b59a 100644
--- a/drivers/gpu/drm/drm_sysfs.c
+++ b/drivers/gpu/drm/drm_sysfs.c
@@ -407,17 +407,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

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH RESEND 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 33466999b59a..674c3df56ea0 100644
--- a/drivers/gpu/drm/drm_sysfs.c
+++ b/drivers/gpu/drm/drm_sysfs.c
@@ -401,7 +401,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
@@ -416,15 +416,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:
@@ -452,14 +454,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 RESEND 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 674c3df56ea0..34e4fbcf44ba 100644
--- a/drivers/gpu/drm/drm_sysfs.c
+++ b/drivers/gpu/drm/drm_sysfs.c
@@ -302,33 +302,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;
@@ -338,38 +333,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;
@@ -379,8 +426,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));
 }
 
@@ -397,13 +443,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
 };
 
@@ -460,7 +514,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 RESEND 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 34e4fbcf44ba..487ddf5ffe51 100644
--- a/drivers/gpu/drm/drm_sysfs.c
+++ b/drivers/gpu/drm/drm_sysfs.c
@@ -312,17 +312,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");
@@ -347,17 +336,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");
@@ -382,11 +360,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");
@@ -411,11 +384,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

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[shuang.he]

Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6387
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                                  276/276              276/276
ILK                                  302/302              302/302
SNB                 -1              314/314              313/314
IVB                                  338/338              338/338
BYT                                  286/286              286/286
BDW                                  320/320              320/320
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
 SNB  igt@pm_rpm@dpms-mode-unset-non-lpsp      DMESG_WARN(13)PASS(1)      DMESG_WARN(1)
(dmesg patch applied)WARNING:at_drivers/gpu/drm/i915/intel_uncore.c:#assert_device_not_suspended[i915]()@WARNING:.* at .* assert_device_not_suspended+0x
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Ville Syrjälä]

On Tue, May 12, 2015 at 12:14:52PM +0300, Jani Nikula wrote:
> This reduces duplication in the patches to follow. No functional
> changes.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

The series looks reasonable.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.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 ffc305fc2076..33466999b59a 100644
> --- a/drivers/gpu/drm/drm_sysfs.c
> +++ b/drivers/gpu/drm/drm_sysfs.c
> @@ -407,17 +407,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

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH RESEND 1/4] drm/sysfs: add a helper for extracting connector type from kobject

[Daniel Vetter]

On Tue, May 12, 2015 at 02:03:36PM +0300, Ville Syrjälä wrote:
> On Tue, May 12, 2015 at 12:14:52PM +0300, Jani Nikula wrote:
> > This reduces duplication in the patches to follow. No functional
> > changes.
> > 
> > Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> 
> The series looks reasonable.
> 
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Series merged to topic/drm-misc, thanks.
-Daniel

> 
> > ---
> >  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 ffc305fc2076..33466999b59a 100644
> > --- a/drivers/gpu/drm/drm_sysfs.c
> > +++ b/drivers/gpu/drm/drm_sysfs.c
> > @@ -407,17 +407,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
> 
> -- 
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel