[Gta04-owner] Has dss 'name' attribute fix been discussed upstream.

Dr. H. Nikolaus Schaller hns at goldelico.com
Tue Feb 24 09:16:09 CET 2015


Hi,

Am 24.02.2015 um 08:55 schrieb NeilBrown <neilb at suse.de>:

> On Tue, 24 Feb 2015 08:31:22 +0100 "Dr. H. Nikolaus Schaller"
> <hns at goldelico.com> wrote:
> 
>> Hi Neil,
>> 
>> Am 24.02.2015 um 04:10 schrieb NeilBrown <neilb at suse.de>:
>> 
>>> 
>>> Hi Nikolaus,
>>> I note you have a patch in your tree:
>>> 
>>> commit 31d50da901ea07db434ca0a299bf13372b8b5534
>>> Author: H. Nikolaus Schaller <hns at goldelico.com>
>>> Date:   Thu May 22 14:22:27 2014 +0200
>>> 
>>>   dss: restore compatibility to pre-3.13 xorg systems by partially reverting 3
>>> 
>>>   Signed-off-by: H. Nikolaus Schaller <hns at goldelico.com>
>>> 
>> 
>> Yes, it was a hack to get it working - without deeper analysis.
>> 
>>> 
>>> I would like to get a fix for this issue upstream.
>>> Have you discussed this issue at all with Tomi?
>> 
>> I think there was one or two mails and if I remember correctly, it was his hint
>> to know what had been changed.
> 
> Nothing really substantial then - OK.

Yes.

> 
>> 
>>> 
>>> I'm happy to start a discussion, but if something has already been said I'd
>>> like to know all the background before I blunder in.
>>> 
>>> my proposed patch is below.
>>> 
>>> Thanks,
>>> NeilBrown
>>> 
>>> From: NeilBrown <neilb at suse.de>
>>> Date: Mon, 23 Feb 2015 16:10:55 +1100
>>> Subject: [PATCH] OMAPDSS: restore "name" sysfs entry.
>>> 
>>> commit 303e4697e762dc92a40405f4e4b8aac02cd0d70b
>>> Author: Tomi Valkeinen <tomi.valkeinen at ti.com>
>>> Date:   Tue Sep 24 17:00:14 2013 +0300
>>> 
>>>   OMAPDSS: rename display-sysfs 'name‘ entry
>> 
>> maybe you could add the commit hash
> 
> It's right there after the word 'commit' :-)
> 
> 
>> 
>>> 
>>> Broke the X server on my device as it couldn't find the display
>>> any more.
>>> 
>>> That commit claim that 'name' is not compatible with i2c or spi.
>>> i2c does register it own 'name' file, but spi does not, hence my
>>> problem.
>> 
>>> 
>>> So create a special case for i2c: add the name attribute for non-i2c
>>> devices.
>>> 
>> 
>> What I do not clearly understand (from your description) is the relation
>> to i2c. AFAIK, there are no panels using I2C? So what role does i2c
>> play here?
> 
> % grep i2c `git grep -l omapdss_register_display`
> 
> suggests that 
> 
>  drivers/video/fbdev/omap2/displays-new/connector-dvi.c
> 
> is an i2c device which devices as a dss display.

Ah, ok. Now I understand.

DVI / HDMI displays have an I2C bus to talk to the display.

And perhaps this interface presents the ‚name‘ attribute so that 
the display name is read from the real display device. And therefore
it was in conflict.

Such a mechanism does not exist for all non-I2C interfaces.
I.e. RGB (as we use it) and MIPI (e.g. for the Pyra).

So the distinction probably more related to DVI/HDMI vs. non-DVI/HDMI
than SPI vs. I2C but finding no i2c interface is probably a good indicator.

Tomi is more specialist on that and I can only give a hint what he might
want to discuss about.

BR,
Nikolaus

> 
> That was added Fri May 24 14:20:45 2013 +0300
> 
> The 'name' attribute was changed
> 
>   Tue Sep 24 17:00:14 2013 +0300
> 
> 4 months later.  There could be a connection.
> 
> 
> 
>> 
>> And we must take care that it does not break on systems we don’t know.
> 
> Always a problem ... and of course that is what the offending patch did.
> 
> Thanks,
> NeilBrown
> 
> 
>> 
>> for those I know it is:
>> GTA04:	RGB, SPI
>> OpenPandora:	RGB, SPI
>> L3704/L7004:	RGB, neither I2C nor SPI
>> Pyra: MIPI, neither I2C nor SPI
>> 
>> BR,
>> Nikolaus
>> 
>> 
>>> Signed-off-by: NeilBrown <neilb at suse.de>
>>> 
>>> diff --git a/drivers/video/fbdev/omap2/dss/display-sysfs.c b/drivers/video/fbdev/omap2/dss/display-sysfs.c
>>> index 5a2095a98ed8..dff6c6764eec 100644
>>> --- a/drivers/video/fbdev/omap2/dss/display-sysfs.c
>>> +++ b/drivers/video/fbdev/omap2/dss/display-sysfs.c
>>> @@ -24,6 +24,7 @@
>>> #include <linux/module.h>
>>> #include <linux/platform_device.h>
>>> #include <linux/sysfs.h>
>>> +#include <linux/i2c.h>
>>> 
>>> #include <video/omapdss.h>
>>> #include "dss.h"
>>> @@ -278,6 +279,7 @@ static ssize_t display_wss_store(struct device *dev,
>>> }
>>> 
>>> static DEVICE_ATTR(display_name, S_IRUGO, display_name_show, NULL);
>>> +static DEVICE_ATTR(name, S_IRUGO, display_name_show, NULL);
>>> static DEVICE_ATTR(enabled, S_IRUGO|S_IWUSR,
>>> 		display_enabled_show, display_enabled_store);
>>> static DEVICE_ATTR(tear_elim, S_IRUGO|S_IWUSR,
>>> @@ -315,6 +317,17 @@ int display_init_sysfs(struct platform_device *pdev)
>>> 			DSSERR("failed to create sysfs files\n");
>>> 			goto err;
>>> 		}
>>> +		if (!i2c_verify_client(dssdev->dev)) {
>>> +			/*
>>> +			 * Add 'name' file - not compatible with i2c, but
>>> +			 * need by Xorg for other devices.
>>> +			 */
>>> +			r = sysfs_create_file(kobj, &dev_attr_name.attr);
>>> +			if (r) {
>>> +				DSSERR("fail to create 'name' sysfs file\n");
>>> +				goto err;
>>> +			}
>>> +		}
>>> 
>>> 		r = sysfs_create_link(&pdev->dev.kobj, kobj, dssdev->alias);
>>> 		if (r) {
>>> @@ -341,5 +354,7 @@ void display_uninit_sysfs(struct platform_device *pdev)
>>> 		sysfs_remove_link(&pdev->dev.kobj, dssdev->alias);
>>> 		sysfs_remove_files(&dssdev->dev->kobj,
>>> 				display_sysfs_attrs);
>>> +		sysfs_remove_file(&dssdev->dev->kobj,
>>> +				  &dev_attr_name.attr);
>>> 	}
>>> }



More information about the Gta04-owner mailing list