* Re: [PATCH v2 2/4] iio: add LM3533 ambient light sensor driver
From: Jonathan Cameron @ 2012-05-19 8:04 UTC (permalink / raw)
To: Johan Hovold
Cc: Jonathan Cameron, Rob Landley, Richard Purdie, Samuel Ortiz,
Greg Kroah-Hartman, Florian Tobias Schandinat, Arnd Bergmann,
Andrew Morton, Mark Brown, linux-doc, linux-kernel, linux-iio,
devel, linux-fbdev
In-Reply-To: <20120518175742.GA12894@localhost>
On 05/18/2012 06:57 PM, Johan Hovold wrote:
> On Fri, May 18, 2012 at 06:34:01PM +0100, Jonathan Cameron wrote:
>> On 05/18/2012 01:27 PM, Johan Hovold wrote:
>
> [...]
>
>>> I really think that this should remain a device specific attribute as I
>>> originally suggested. It's an integration parameter that needs to be set
>>> precisely depending on the actual hardware setup (which analog light
>>> sensor and other external components).
>> Then it shouldn't be exposed to userspace. If there is reason to vary
>> it from userspace then it is a calibration parameter and should be
>> treated like the other ones we have, if not it should be done from
>> dt or platform data.
>>>
>>> The lm3533 also supports two types of light sensors: pwm- and analog-
>>> output ones. The resistor select settings only applies when in analog
>>> mode as the input is always high impedance otherwise. Thus a generic
>>> attribute (such as calibscale or hardware gain) shouldn't be used as it
>>> will have no effect whatsoever in PWM-mode.
>>>
>>> I'm thus back at my original proposal, albeit with a different name (I
>>> think a lot of this discussion could have been avoided had I not
>>> misnamed the parameter "gain"):
>>>
>>> What: /sys/bus/iio/devices/iio:deviceX/r_select
>>> Description:
>>> Set the ALS internal pull-down resistor for analog input mode
>>> (1..127), such that,
>>>
>>> R_als = 200000 / r_select (ohm)
>>>
>>> This setting is ignored in PWM-mode (input is always high
>>> impedance in PWM-mode).
>>>
>>> I don't think much is gained from using ohm as the unit: it just adds
>>> complexity and the selected resistor setting will likely not match the
>>> input value anyway. It's better that the chip integrators have full
>>> control over which resistor setting is actually used so that it matches
>>> external components.
>> This smacks of something that should never be exposed to users.
>> I'd hide it away in platform data.
>
> Fair enough. I'll drop the sysfs param and submit a patch for mfd-next
> which adds r_select to the platform data.
>
cool. I'll review the rest of the patch with the assumption you'll do this.
Jonathan
^ permalink raw reply
* [PATCH] backlight: initialize struct backlight_properties properly
From: Corentin Chary @ 2012-05-21 7:24 UTC (permalink / raw)
To: Richard Purdie
Cc: Corentin Chary, Keith Packard, David Airlie, Matthew Garrett,
Florian Tobias Schandinat, Mark Brown, Tomi Valkeinen,
Mathieu Poirier, Dave Airlie, Arnd Bergmann, Lars-Peter Clausen,
Axel Lin, dri-devel, linux-kernel, platform-driver-x86,
linux-fbdev, patches, linux-omap
In all these files, the .power field was never correctly initialized.
Signed-off-by: Corentin Chary <corentin.chary@gmail.com>
---
drivers/gpu/drm/i915/intel_panel.c | 1 +
drivers/gpu/drm/radeon/radeon_legacy_encoders.c | 1 +
drivers/platform/x86/toshiba_acpi.c | 1 +
drivers/video/backlight/da903x_bl.c | 1 +
drivers/video/backlight/pcf50633-backlight.c | 1 +
drivers/video/backlight/wm831x_bl.c | 1 +
drivers/video/omap2/displays/panel-acx565akm.c | 1 +
7 files changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index 48177ec..ec4a1e0 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -342,6 +342,7 @@ int intel_panel_setup_backlight(struct drm_device *dev)
else
return -ENODEV;
+ memset(&props, 0, sizeof(props));
props.type = BACKLIGHT_RAW;
props.max_brightness = intel_panel_get_max_backlight(dev);
dev_priv->backlight diff --git a/drivers/gpu/drm/radeon/radeon_legacy_encoders.c b/drivers/gpu/drm/radeon/radeon_legacy_encoders.c
index 42db254..a0c8222 100644
--- a/drivers/gpu/drm/radeon/radeon_legacy_encoders.c
+++ b/drivers/gpu/drm/radeon/radeon_legacy_encoders.c
@@ -369,6 +369,7 @@ void radeon_legacy_backlight_init(struct radeon_encoder *radeon_encoder,
goto error;
}
+ memset(&props, 0, sizeof(props));
props.max_brightness = MAX_RADEON_LEVEL;
props.type = BACKLIGHT_RAW;
bd = backlight_device_register("radeon_bl", &drm_connector->kdev,
diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
index bde32de..d680bb2 100644
--- a/drivers/platform/x86/toshiba_acpi.c
+++ b/drivers/platform/x86/toshiba_acpi.c
@@ -1095,6 +1095,7 @@ static int __devinit toshiba_acpi_setup_backlight(struct toshiba_acpi_dev *dev)
ret = get_tr_backlight_status(dev, &enabled);
dev->tr_backlight_supported = !ret;
+ memset(&props, 0, sizeof(props));
props.type = BACKLIGHT_PLATFORM;
props.max_brightness = HCI_LCD_BRIGHTNESS_LEVELS - 1;
diff --git a/drivers/video/backlight/da903x_bl.c b/drivers/video/backlight/da903x_bl.c
index 30e1968..573c7ec 100644
--- a/drivers/video/backlight/da903x_bl.c
+++ b/drivers/video/backlight/da903x_bl.c
@@ -136,6 +136,7 @@ static int da903x_backlight_probe(struct platform_device *pdev)
da903x_write(data->da903x_dev, DA9034_WLED_CONTROL2,
DA9034_WLED_ISET(pdata->output_current));
+ memset(&props, 0, sizeof(props));
props.type = BACKLIGHT_RAW;
props.max_brightness = max_brightness;
bl = backlight_device_register(pdev->name, data->da903x_dev, data,
diff --git a/drivers/video/backlight/pcf50633-backlight.c b/drivers/video/backlight/pcf50633-backlight.c
index c65853c..c092159 100644
--- a/drivers/video/backlight/pcf50633-backlight.c
+++ b/drivers/video/backlight/pcf50633-backlight.c
@@ -111,6 +111,7 @@ static int __devinit pcf50633_bl_probe(struct platform_device *pdev)
if (!pcf_bl)
return -ENOMEM;
+ memset(&bl_props, 0, sizeof(bl_props));
bl_props.type = BACKLIGHT_RAW;
bl_props.max_brightness = 0x3f;
bl_props.power = FB_BLANK_UNBLANK;
diff --git a/drivers/video/backlight/wm831x_bl.c b/drivers/video/backlight/wm831x_bl.c
index 5d365de..9e5517a 100644
--- a/drivers/video/backlight/wm831x_bl.c
+++ b/drivers/video/backlight/wm831x_bl.c
@@ -194,6 +194,7 @@ static int wm831x_backlight_probe(struct platform_device *pdev)
data->current_brightness = 0;
data->isink_reg = isink_reg;
+ memset(&props, 0, sizeof(props));
props.type = BACKLIGHT_RAW;
props.max_brightness = max_isel;
bl = backlight_device_register("wm831x", &pdev->dev, data,
diff --git a/drivers/video/omap2/displays/panel-acx565akm.c b/drivers/video/omap2/displays/panel-acx565akm.c
index d26f37a..74e7cf0 100644
--- a/drivers/video/omap2/displays/panel-acx565akm.c
+++ b/drivers/video/omap2/displays/panel-acx565akm.c
@@ -532,6 +532,7 @@ static int acx_panel_probe(struct omap_dss_device *dssdev)
/*------- Backlight control --------*/
+ memset(&props, 0, sizeof(props));
props.fb_blank = FB_BLANK_UNBLANK;
props.power = FB_BLANK_UNBLANK;
props.type = BACKLIGHT_RAW;
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH] OMAPDSS: DSI: Support command mode interleaving during video mode blanking periods
From: Archit Taneja @ 2012-05-21 8:26 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-fbdev, linux-omap
In-Reply-To: <1337348396.2607.19.camel@deskari>
On Friday 18 May 2012 07:09 PM, Tomi Valkeinen wrote:
> Hi,
>
> On Tue, 2012-05-15 at 11:32 +0530, Archit Taneja wrote:
>> DSI supports interleaving of command mode packets during the HSA, HFP, HBP and
>> BLLP blanking intervals in a video mode stream. This is useful as a user may
>> want to read or change the configuration of a panel without stopping the video
>> stream.
>>
>> On OMAP DSI, we can queue HS or LP command mode packets in the TX FIFO, and
>> the DSI HW takes care of interleaving this data during the one of the blanking
>> intervals. The DSI HW needs to be programmed with the maximum amount of data
>> that can be interleaved in a particular blanking period. A blanking period
>> cannot be used to send command mode data for it's complete duration, there is
>> some amount of time required for the DSI data and clock lanes to transition
>> to the desired LP or HS state.
>>
>> Based on the state of the lanes at the beginning and end of the blanking period,
>> we have different scenarios, with each scenario having a different value of time
>> required to transition to HS or LP. Refer to the section 'Interleaving Mode' in
>> OMAP TRM for more info on the scenarios and the equations to calculate the time
>> required for HS or LP transitions.
>>
>> We use the scenarios which takes the maximum time for HS or LP transition, this
>> gives us the minimum amount of time that can be used to interleave command mode
>> data. The amount of data that can be sent during this minimum time is calculated
>> for command mode packets both in LP and HS. These are written to the registers
>> DSI_VM_TIMING4 to DSI_VM_TIMING6.
>>
>> The calculations don't take into account the time required of transmitting BTA
>> when doing a DSI read, or verifying if a DSI write went through correctly. Until
>> these latencies aren't considered, the behaviour of DSI is unpredictable when
>> a BTA is interleaved during a blanking period. Enhancement of these calculations
>> is a TODO item.
>
> The code in dsi_config_cmd_mode_interleaving() looks a bit long and
> confusing, but I don't think it's trivial to simplify it. There's just
> so many variables to consider.
There is a little optimisation done in the code to not calculate the
interleaving parameter for a blanking period which is not used for
interleaving. Checks like below are done for that:
if(!hsa_blanking_mode) {
...
}
if (!hfp_blanking_mode) {
...
}
...
We could remove this, as the computation is just done once and there
seems to be no side effects to write interleaving parameters for a
blanking period not used for interleaving. But it won't make much
function much simpler.
>
> In the future I think we should store the DSI configurations into
> memory, so that we don't need to parse the hardware registers to find
> out things like DSI timings.
Yes, I'll do this clean up sometime soon.
Archit
^ permalink raw reply
* Re: [PATCH] backlight: initialize struct backlight_properties properly
From: Jingoo Han @ 2012-05-22 0:07 UTC (permalink / raw)
To: 'Corentin Chary', 'Richard Purdie',
'Andrew Morton'
Cc: 'Keith Packard', 'David Airlie',
'Matthew Garrett', 'Florian Tobias Schandinat',
'Mark Brown', 'Tomi Valkeinen',
'Mathieu Poirier', 'Dave Airlie',
'Arnd Bergmann', 'Lars-Peter Clausen',
'Axel Lin', dri-devel, linux-kernel, platform-driver-x86,
linux-fbdev, patches, linux-omap, 'Jingoo Han'
In-Reply-To: <1337585085-4891-1-git-send-email-corentin.chary@gmail.com>
On Mon, May 21, 2012 at 04:25 PM +0000, Corentin Chary wrote:
>
> In all these files, the .power field was never correctly initialized.
>
> Signed-off-by: Corentin Chary <corentin.chary@gmail.com>
It looks good.
Best regards,
Jingoo Han
> ---
> drivers/gpu/drm/i915/intel_panel.c | 1 +
> drivers/gpu/drm/radeon/radeon_legacy_encoders.c | 1 +
> drivers/platform/x86/toshiba_acpi.c | 1 +
> drivers/video/backlight/da903x_bl.c | 1 +
> drivers/video/backlight/pcf50633-backlight.c | 1 +
> drivers/video/backlight/wm831x_bl.c | 1 +
> drivers/video/omap2/displays/panel-acx565akm.c | 1 +
> 7 files changed, 7 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index 48177ec..ec4a1e0 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -342,6 +342,7 @@ int intel_panel_setup_backlight(struct drm_device *dev)
> else
> return -ENODEV;
>
> + memset(&props, 0, sizeof(props));
> props.type = BACKLIGHT_RAW;
> props.max_brightness = intel_panel_get_max_backlight(dev);
> dev_priv->backlight > diff --git a/drivers/gpu/drm/radeon/radeon_legacy_encoders.c
> b/drivers/gpu/drm/radeon/radeon_legacy_encoders.c
> index 42db254..a0c8222 100644
> --- a/drivers/gpu/drm/radeon/radeon_legacy_encoders.c
> +++ b/drivers/gpu/drm/radeon/radeon_legacy_encoders.c
> @@ -369,6 +369,7 @@ void radeon_legacy_backlight_init(struct radeon_encoder *radeon_encoder,
> goto error;
> }
>
> + memset(&props, 0, sizeof(props));
> props.max_brightness = MAX_RADEON_LEVEL;
> props.type = BACKLIGHT_RAW;
> bd = backlight_device_register("radeon_bl", &drm_connector->kdev,
> diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
> index bde32de..d680bb2 100644
> --- a/drivers/platform/x86/toshiba_acpi.c
> +++ b/drivers/platform/x86/toshiba_acpi.c
> @@ -1095,6 +1095,7 @@ static int __devinit toshiba_acpi_setup_backlight(struct toshiba_acpi_dev *dev)
> ret = get_tr_backlight_status(dev, &enabled);
> dev->tr_backlight_supported = !ret;
>
> + memset(&props, 0, sizeof(props));
> props.type = BACKLIGHT_PLATFORM;
> props.max_brightness = HCI_LCD_BRIGHTNESS_LEVELS - 1;
>
> diff --git a/drivers/video/backlight/da903x_bl.c b/drivers/video/backlight/da903x_bl.c
> index 30e1968..573c7ec 100644
> --- a/drivers/video/backlight/da903x_bl.c
> +++ b/drivers/video/backlight/da903x_bl.c
> @@ -136,6 +136,7 @@ static int da903x_backlight_probe(struct platform_device *pdev)
> da903x_write(data->da903x_dev, DA9034_WLED_CONTROL2,
> DA9034_WLED_ISET(pdata->output_current));
>
> + memset(&props, 0, sizeof(props));
> props.type = BACKLIGHT_RAW;
> props.max_brightness = max_brightness;
> bl = backlight_device_register(pdev->name, data->da903x_dev, data,
> diff --git a/drivers/video/backlight/pcf50633-backlight.c b/drivers/video/backlight/pcf50633-backlight.c
> index c65853c..c092159 100644
> --- a/drivers/video/backlight/pcf50633-backlight.c
> +++ b/drivers/video/backlight/pcf50633-backlight.c
> @@ -111,6 +111,7 @@ static int __devinit pcf50633_bl_probe(struct platform_device *pdev)
> if (!pcf_bl)
> return -ENOMEM;
>
> + memset(&bl_props, 0, sizeof(bl_props));
> bl_props.type = BACKLIGHT_RAW;
> bl_props.max_brightness = 0x3f;
> bl_props.power = FB_BLANK_UNBLANK;
> diff --git a/drivers/video/backlight/wm831x_bl.c b/drivers/video/backlight/wm831x_bl.c
> index 5d365de..9e5517a 100644
> --- a/drivers/video/backlight/wm831x_bl.c
> +++ b/drivers/video/backlight/wm831x_bl.c
> @@ -194,6 +194,7 @@ static int wm831x_backlight_probe(struct platform_device *pdev)
> data->current_brightness = 0;
> data->isink_reg = isink_reg;
>
> + memset(&props, 0, sizeof(props));
> props.type = BACKLIGHT_RAW;
> props.max_brightness = max_isel;
> bl = backlight_device_register("wm831x", &pdev->dev, data,
> diff --git a/drivers/video/omap2/displays/panel-acx565akm.c b/drivers/video/omap2/displays/panel-
> acx565akm.c
> index d26f37a..74e7cf0 100644
> --- a/drivers/video/omap2/displays/panel-acx565akm.c
> +++ b/drivers/video/omap2/displays/panel-acx565akm.c
> @@ -532,6 +532,7 @@ static int acx_panel_probe(struct omap_dss_device *dssdev)
>
> /*------- Backlight control --------*/
>
> + memset(&props, 0, sizeof(props));
> props.fb_blank = FB_BLANK_UNBLANK;
> props.power = FB_BLANK_UNBLANK;
> props.type = BACKLIGHT_RAW;
> --
> 1.7.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] backlight: initialize struct backlight_properties properly
From: Dave Airlie @ 2012-05-22 9:25 UTC (permalink / raw)
To: Corentin Chary
Cc: Richard Purdie, Keith Packard, David Airlie, Matthew Garrett,
Florian Tobias Schandinat, Mark Brown, Tomi Valkeinen,
Mathieu Poirier, Dave Airlie, Arnd Bergmann, Lars-Peter Clausen,
Axel Lin, dri-devel, linux-kernel, platform-driver-x86,
linux-fbdev, patches, linux-omap
In-Reply-To: <1337585085-4891-1-git-send-email-corentin.chary@gmail.com>
On Mon, May 21, 2012 at 8:24 AM, Corentin Chary
<corentin.chary@gmail.com> wrote:
> In all these files, the .power field was never correctly initialized.
>
> Signed-off-by: Corentin Chary <corentin.chary@gmail.com>
> ---
> drivers/gpu/drm/i915/intel_panel.c | 1 +
> drivers/gpu/drm/radeon/radeon_legacy_encoders.c | 1 +
I've taken these two hunks into drm-next.
Dave.
^ permalink raw reply
* [PATCH 1/2] leds: lm3533: replace als attribute with als_channel and als_en
From: Johan Hovold @ 2012-05-22 10:05 UTC (permalink / raw)
To: Richard Purdie, Andrew Morton
Cc: Rob Landley, Florian Tobias Schandinat, linux-doc, linux-fbdev,
Jonathan Cameron, linux-kernel, Johan Hovold
Replace the als attribute with two separate attributes for selecting ALS
channel and enabling ALS-current-control mode.
This change is needed to reflect changes made to the ALS sub-driver
which now uses 0-indexed current output channels (rather than 1-indexed
ALS-mapper target sets).
Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
.../ABI/testing/sysfs-class-led-driver-lm3533 | 19 +++-
drivers/leds/leds-lm3533.c | 98 +++++++++++++++-----
2 files changed, 87 insertions(+), 30 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-class-led-driver-lm3533 b/Documentation/ABI/testing/sysfs-class-led-driver-lm3533
index a9633fd..620ebb3 100644
--- a/Documentation/ABI/testing/sysfs-class-led-driver-lm3533
+++ b/Documentation/ABI/testing/sysfs-class-led-driver-lm3533
@@ -1,13 +1,20 @@
-What: /sys/class/leds/<led>/als
-Date: April 2012
+What: /sys/class/leds/<led>/als_channel
+Date: May 2012
KernelVersion: 3.5
Contact: Johan Hovold <jhovold@gmail.com>
Description:
- Set the ALS-control mode (0, 2, 3), where
+ Set the ALS output channel to use as input in
+ ALS-current-control mode (1, 2), where
+
+ 1 - out_current1
+ 2 - out_current2
- 0 - disabled
- 2 - ALS-mapper 2
- 3 - ALS-mapper 3
+What: /sys/class/leds/<led>/als_en
+Date: May 2012
+KernelVersion: 3.5
+Contact: Johan Hovold <jhovold@gmail.com>
+Description:
+ Enable ALS-current-control mode (0, 1).
What: /sys/class/leds/<led>/falltime
What: /sys/class/leds/<led>/risetime
diff --git a/drivers/leds/leds-lm3533.c b/drivers/leds/leds-lm3533.c
index e968470..f56b6e7 100644
--- a/drivers/leds/leds-lm3533.c
+++ b/drivers/leds/leds-lm3533.c
@@ -27,8 +27,8 @@
#define LM3533_LVCTRLBANK_MAX 5
#define LM3533_LVCTRLBANK_COUNT 4
#define LM3533_RISEFALLTIME_MAX 7
-#define LM3533_ALS_LV_MIN 2
-#define LM3533_ALS_LV_MAX 3
+#define LM3533_ALS_CHANNEL_LV_MIN 1
+#define LM3533_ALS_CHANNEL_LV_MAX 2
#define LM3533_REG_CTRLBANK_BCONF_BASE 0x1b
#define LM3533_REG_PATTERN_ENABLE 0x28
@@ -39,8 +39,9 @@
#define LM3533_REG_PATTERN_STEP 0x10
-#define LM3533_REG_CTRLBANK_BCONF_MAPPING_MASK 0x04
-#define LM3533_REG_CTRLBANK_BCONF_ALS_MASK 0x03
+#define LM3533_REG_CTRLBANK_BCONF_MAPPING_MASK 0x04
+#define LM3533_REG_CTRLBANK_BCONF_ALS_EN_MASK 0x02
+#define LM3533_REG_CTRLBANK_BCONF_ALS_CHANNEL_MASK 0x01
#define LM3533_LED_FLAG_PATTERN_ENABLE 1
@@ -416,21 +417,14 @@ static ssize_t store_falltime(struct device *dev,
LM3533_REG_PATTERN_FALLTIME_BASE);
}
-/*
- * ALS-control setting:
- *
- * 0 - ALS disabled
- * 2 - ALS-mapper 2
- * 3 - ALS-mapper 3
- */
-static ssize_t show_als(struct device *dev,
+static ssize_t show_als_channel(struct device *dev,
struct device_attribute *attr, char *buf)
{
struct led_classdev *led_cdev = dev_get_drvdata(dev);
struct lm3533_led *led = to_lm3533_led(led_cdev);
+ unsigned channel;
u8 reg;
u8 val;
- int als;
int ret;
reg = lm3533_led_get_lv_reg(led, LM3533_REG_CTRLBANK_BCONF_BASE);
@@ -438,32 +432,85 @@ static ssize_t show_als(struct device *dev,
if (ret)
return ret;
- als = val & LM3533_REG_CTRLBANK_BCONF_ALS_MASK;
+ channel = (val & LM3533_REG_CTRLBANK_BCONF_ALS_CHANNEL_MASK) + 1;
- return scnprintf(buf, PAGE_SIZE, "%d\n", als);
+ return scnprintf(buf, PAGE_SIZE, "%u\n", channel);
}
-static ssize_t store_als(struct device *dev,
+static ssize_t store_als_channel(struct device *dev,
struct device_attribute *attr,
const char *buf, size_t len)
{
struct led_classdev *led_cdev = dev_get_drvdata(dev);
struct lm3533_led *led = to_lm3533_led(led_cdev);
- u8 als;
+ unsigned channel;
u8 reg;
+ u8 val;
u8 mask;
int ret;
- if (kstrtou8(buf, 0, &als))
+ if (kstrtouint(buf, 0, &channel))
return -EINVAL;
- if (als != 0 && (als < LM3533_ALS_LV_MIN || als > LM3533_ALS_LV_MAX))
+ if (channel < LM3533_ALS_CHANNEL_LV_MIN ||
+ channel > LM3533_ALS_CHANNEL_LV_MAX)
return -EINVAL;
reg = lm3533_led_get_lv_reg(led, LM3533_REG_CTRLBANK_BCONF_BASE);
- mask = LM3533_REG_CTRLBANK_BCONF_ALS_MASK;
+ mask = LM3533_REG_CTRLBANK_BCONF_ALS_CHANNEL_MASK;
+ val = channel - 1;
+
+ ret = lm3533_update(led->lm3533, reg, val, mask);
+ if (ret)
+ return ret;
+
+ return len;
+}
+
+static ssize_t show_als_en(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct led_classdev *led_cdev = dev_get_drvdata(dev);
+ struct lm3533_led *led = to_lm3533_led(led_cdev);
+ bool enable;
+ u8 reg;
+ u8 val;
+ int ret;
+
+ reg = lm3533_led_get_lv_reg(led, LM3533_REG_CTRLBANK_BCONF_BASE);
+ ret = lm3533_read(led->lm3533, reg, &val);
+ if (ret)
+ return ret;
- ret = lm3533_update(led->lm3533, reg, als, mask);
+ enable = val & LM3533_REG_CTRLBANK_BCONF_ALS_EN_MASK;
+
+ return scnprintf(buf, PAGE_SIZE, "%d\n", enable);
+}
+
+static ssize_t store_als_en(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t len)
+{
+ struct led_classdev *led_cdev = dev_get_drvdata(dev);
+ struct lm3533_led *led = to_lm3533_led(led_cdev);
+ unsigned enable;
+ u8 reg;
+ u8 mask;
+ u8 val;
+ int ret;
+
+ if (kstrtouint(buf, 0, &enable))
+ return -EINVAL;
+
+ reg = lm3533_led_get_lv_reg(led, LM3533_REG_CTRLBANK_BCONF_BASE);
+ mask = LM3533_REG_CTRLBANK_BCONF_ALS_EN_MASK;
+
+ if (enable)
+ val = mask;
+ else
+ val = 0;
+
+ ret = lm3533_update(led->lm3533, reg, val, mask);
if (ret)
return ret;
@@ -558,7 +605,8 @@ static ssize_t store_pwm(struct device *dev,
return len;
}
-static LM3533_ATTR_RW(als);
+static LM3533_ATTR_RW(als_channel);
+static LM3533_ATTR_RW(als_en);
static LM3533_ATTR_RW(falltime);
static LM3533_ATTR_RO(id);
static LM3533_ATTR_RW(linear);
@@ -566,7 +614,8 @@ static LM3533_ATTR_RW(pwm);
static LM3533_ATTR_RW(risetime);
static struct attribute *lm3533_led_attributes[] = {
- &dev_attr_als.attr,
+ &dev_attr_als_channel.attr,
+ &dev_attr_als_en.attr,
&dev_attr_falltime.attr,
&dev_attr_id.attr,
&dev_attr_linear.attr,
@@ -583,7 +632,8 @@ static umode_t lm3533_led_attr_is_visible(struct kobject *kobj,
struct lm3533_led *led = to_lm3533_led(led_cdev);
umode_t mode = attr->mode;
- if (attr = &dev_attr_als.attr) {
+ if (attr = &dev_attr_als_channel.attr ||
+ attr = &dev_attr_als_en.attr) {
if (!led->lm3533->have_als)
mode = 0;
}
--
1.7.8.5
^ permalink raw reply related
* [PATCH 2/2] backlight: lm3533: replace als attribute with als_channel and als_en
From: Johan Hovold @ 2012-05-22 10:05 UTC (permalink / raw)
To: Richard Purdie, Andrew Morton
Cc: Rob Landley, Florian Tobias Schandinat, linux-doc, linux-fbdev,
Jonathan Cameron, linux-kernel, Johan Hovold
In-Reply-To: <1337681108-13838-1-git-send-email-jhovold@gmail.com>
Replace the als attribute with two separate attributes for selecting ALS
channel and enabling ALS-current-control mode.
This change is needed to reflect changes made to the ALS sub-driver
which now uses 0-indexed current output channels (rather than 1-indexed
ALS-mapper target sets).
Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
.../testing/sysfs-class-backlight-driver-lm3533 | 19 +++++---
drivers/video/backlight/lm3533_bl.c | 48 ++++++++++----------
2 files changed, 37 insertions(+), 30 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-class-backlight-driver-lm3533 b/Documentation/ABI/testing/sysfs-class-backlight-driver-lm3533
index ea91f71..77cf7ac 100644
--- a/Documentation/ABI/testing/sysfs-class-backlight-driver-lm3533
+++ b/Documentation/ABI/testing/sysfs-class-backlight-driver-lm3533
@@ -1,13 +1,20 @@
-What: /sys/class/backlight/<backlight>/als
-Date: April 2012
+What: /sys/class/backlight/<backlight>/als_channel
+Date: May 2012
KernelVersion: 3.5
Contact: Johan Hovold <jhovold@gmail.com>
Description:
- Set the ALS-control mode (0..2), where
+ Get the ALS output channel used as input in
+ ALS-current-control mode (0, 1), where
+
+ 0 - out_current0 (backlight 0)
+ 1 - out_current1 (backlight 1)
- 0 - disabled
- 1 - ALS-mapper 1 (backlight 0)
- 2 - ALS-mapper 2 (backlight 1)
+What: /sys/class/backlight/<backlight>/als_en
+Date: May 2012
+KernelVersion: 3.5
+Contact: Johan Hovold <jhovold@gmail.com>
+Description:
+ Enable ALS-current-control mode (0, 1).
What: /sys/class/backlight/<backlight>/id
Date: April 2012
diff --git a/drivers/video/backlight/lm3533_bl.c b/drivers/video/backlight/lm3533_bl.c
index d916ffe..18dca0c 100644
--- a/drivers/video/backlight/lm3533_bl.c
+++ b/drivers/video/backlight/lm3533_bl.c
@@ -79,55 +79,52 @@ static ssize_t show_id(struct device *dev,
return scnprintf(buf, PAGE_SIZE, "%d\n", bl->id);
}
-/*
- * ALS-control setting:
- *
- * 0 - ALS disabled
- * 1 - ALS-mapper 1 (backlight 0)
- * 2 - ALS-mapper 2 (backlight 1)
- */
-static ssize_t show_als(struct device *dev,
+static ssize_t show_als_channel(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct lm3533_bl *bl = dev_get_drvdata(dev);
+ unsigned channel = lm3533_bl_get_ctrlbank_id(bl);
+
+ return scnprintf(buf, PAGE_SIZE, "%u\n", channel);
+}
+
+static ssize_t show_als_en(struct device *dev,
struct device_attribute *attr, char *buf)
{
struct lm3533_bl *bl = dev_get_drvdata(dev);
int ctrlbank = lm3533_bl_get_ctrlbank_id(bl);
u8 val;
u8 mask;
- int als;
+ bool enable;
int ret;
ret = lm3533_read(bl->lm3533, LM3533_REG_CTRLBANK_AB_BCONF, &val);
if (ret)
return ret;
- mask = 2 * ctrlbank;
- als = val & mask;
- if (als)
- als = ctrlbank + 1;
+ mask = 1 << (2 * ctrlbank);
+ enable = val & mask;
- return scnprintf(buf, PAGE_SIZE, "%d\n", als);
+ return scnprintf(buf, PAGE_SIZE, "%d\n", enable);
}
-static ssize_t store_als(struct device *dev,
+static ssize_t store_als_en(struct device *dev,
struct device_attribute *attr,
const char *buf, size_t len)
{
struct lm3533_bl *bl = dev_get_drvdata(dev);
int ctrlbank = lm3533_bl_get_ctrlbank_id(bl);
- int als;
+ int enable;
u8 val;
u8 mask;
int ret;
- if (kstrtoint(buf, 0, &als))
- return -EINVAL;
-
- if (als != 0 && (als != ctrlbank + 1))
+ if (kstrtoint(buf, 0, &enable))
return -EINVAL;
mask = 1 << (2 * ctrlbank);
- if (als)
+ if (enable)
val = mask;
else
val = 0;
@@ -224,13 +221,15 @@ static ssize_t store_pwm(struct device *dev,
return len;
}
-static LM3533_ATTR_RW(als);
+static LM3533_ATTR_RO(als_channel);
+static LM3533_ATTR_RW(als_en);
static LM3533_ATTR_RO(id);
static LM3533_ATTR_RW(linear);
static LM3533_ATTR_RW(pwm);
static struct attribute *lm3533_bl_attributes[] = {
- &dev_attr_als.attr,
+ &dev_attr_als_channel.attr,
+ &dev_attr_als_en.attr,
&dev_attr_id.attr,
&dev_attr_linear.attr,
&dev_attr_pwm.attr,
@@ -244,7 +243,8 @@ static umode_t lm3533_bl_attr_is_visible(struct kobject *kobj,
struct lm3533_bl *bl = dev_get_drvdata(dev);
umode_t mode = attr->mode;
- if (attr = &dev_attr_als.attr) {
+ if (attr = &dev_attr_als_channel.attr ||
+ attr = &dev_attr_als_en.attr) {
if (!bl->lm3533->have_als)
mode = 0;
}
--
1.7.8.5
^ permalink raw reply related
* [GIT PULL] OMAP DSS for v3.5
From: Tomi Valkeinen @ 2012-05-22 10:09 UTC (permalink / raw)
To: Florian Tobias Schandinat; +Cc: linux-omap, linux-fbdev, Tony Lindgren
[-- Attachment #1: Type: text/plain, Size: 11032 bytes --]
Hi Florian,
Here are the OMAP DSS changes for 3.5.
I really tried this time to send the pull request early, but here I am
again, sending it when the merge window has already opened... A few
late-found bugs caused some unnecessary delays.
I'm using github this time instead of gitorious, as gitorious seems to
be down. This is my first pull request with a signed tag, I hope
everything is correct.
Note that there's a merge for small branch with omap board file changes
(e4a9e94cc58ed6e4efb02b80be3a9bf57f448d07) that is also pulled by Tony
for the linux-omap tree. This is meant to help avoid conflicts in the
board files.
Tomi
The following changes since commit 66f75a5d028beaf67c931435fdc3e7823125730c:
Linux 3.4-rc4 (2012-04-21 14:47:52 -0700)
are available in the git repository at:
git://github.com/tomba/linux.git tags/omapdss-for-3.5
for you to fetch changes up to e92a5b28f71aea01b281f9c89d97a4bc5b24748f:
OMAPDSS: HDMI: OMAP4: Update IRQ flags for the HPD IRQ request (2012-05-22 11:00:09 +0300)
----------------------------------------------------------------
Omapdss driver changes for 3.5 merge window.
Lots of normal development commits, but perhaps most notable changes are:
* HDMI rework to properly decouple the HDMI audio part from the HDMI video part.
* Restructure omapdss core driver so that it's possible to implement device
tree support. This included changing how platform data is passed to the
drivers, changing display device registration and improving the panel driver's
ability to configure the underlying video output interface.
* Basic support for DSI packet interleaving
----------------------------------------------------------------
Archit Taneja (19):
OMAPDSS: DISPC/RFBI: Use dispc_mgr_set_lcd_timings() for setting lcd size
OMAPDSS: DISPC: Use a common function to set manager timings
OMAPDSS: DISPC: Clean up manager timing/size functions
OMAPDSS: HDMI: Fix ti_hdmi_4xxx_core_dump
OMAPDSS: HDMI: define and dump CORE registers in correct order
OMAPDSS: Fix DSI_FCLK clock source selection
OMAPDSS: DISPC: Remove Fake VSYNC support
OMAPDSS: APPLY: Add manager timings as extra_info in private data
OMAPDSS: Apply manager timings instead of direct DISPC writes
OMAPDSS: MANAGER: Create a function to check manager timings
OMAPDSS: APPLY: Don't check manager settings if it is disabled
OMAPDSS: APPLY: Remove display dependency from overlay and manager checks
OMAPDSS: DPI/HDMI: Apply manager timings even if panel is disabled
OMAPDSS: APPLY: Remove an unnecessary omap_dss_device pointer
OMAPDSS: DISPC: Remove omap_dss_device pointer usage from dispc_mgr_pclk_rate()
OMAPDSS: DISPC: Remove usage of dispc_mgr_get_device()
OMAPDSS: DSI: Support command mode interleaving during video mode blanking periods
OMAPDSS: VENC/DISPC: Delay dividing Y resolution for managers connected to VENC
OMAPDSS: Apply VENC timings even if panel is disabled
Axel Castaneda Gonzalez (1):
OMAPDSS: HDMI: Decouple wrapper enable/disable and audio start/stop
Chandrabhanu Mahapatra (5):
OMAPDSS: DISPC: Enable predecimation
OMAPDSS: DISPC: Handle synclost errors in OMAP3
OMAPDSS: DISPC: Correct DISPC functional clock usage
OMAPDSS: DISPC: Update Accumulator configuration for chroma plane
OMAPDSS: DISPC: Support rotation through TILER
Grazvydas Ignotas (5):
OMAPDSS: OMAPFB: check for matching memory size early
OMAPDSS: provide default get_timings function for panels
OMAPDSS: TPO-TD03MTEA1: add set/check timing functions
OMAPDSS: VENC: allow switching venc output type at runtime
OMAPDSS: OMAPFB: always allow to configure overlay
Jan Weitzel (1):
ARM: OMAP2+ Add Primview displays to panel-generic
Mark Brown (4):
OMAP: DSS2: Remove suspicous and unused TAAL regulator API usage
OMAPDSS: VENC: Check for errors from regulator_enable()
OMAPDSS: TPO-TD03MTEA1: Check for errors from regulator_enable()
OMAPDSS: TPO-TD03MTEA1: Correct comment for power on delay
Peter Meerwald (1):
OMAPFB: remove unused FB_OMAP_BOOTLOADER_INIT config option
Ricardo Neri (15):
OMAPDSS: Provide an interface for audio support
OMAPDSS: HDMI: Split audio_enable into audio_enable/disable
OMAPDSS: HDMI: Split video_enable into video_enable/disable
OMAPDSS: HDMI: Remove ASoC codec
OMAPDSS: HDMI: OMAP4: Remove CEA-861 audio infoframe and IEC-60958 enums
OMAPDSS: HDMI: OMAP4: Remove invalid I2S settings
OMAPDSS: HDMI: Decouple HDMI audio from ASoC
OMAPDSS: HDMI: OMAP4: Expand configuration for IEC-60958 audio
OMAPDSS: HDMI: Relocate N/CTS calculation
OMAPDSS: HDMI: Add support for more audio sample rates in N/CTS calculation
OMAPDSS: HDMI: Add an audio configuration function
OMAPDSS: HDMI: OMAP4: Remap speaker order to match ALSA order
OMAPDSS: HDMI: Panel: Simplify the name of the HDMI mutex
OMAPDSS: HDMI: Implement DSS driver interface for audio
OMAPDSS: HDMI: OMAP4: Update IRQ flags for the HPD IRQ request
Russ Dill (1):
OMAPDSS: TFP410: use gpio_set_value_cansleep
Thomas Weber (2):
OMAPDSS: Add Mitsubishi AA084SB01 display support
OMAPDSS: Add EDT ET0500G0DH6 display support
Tomi Valkeinen (38):
OMAPDSS: add set_min_bus_tput pointer to omapdss's platform data
OMAPDSS: Ensure OPP100 when DSS is operational
OMAPDSS: DSI: remove option to use pck for DSI PLL clkin
OMAPDSS: panel-dvi: add PD gpio handling
OMAP: board-files: remove custom PD GPIO handling for DVI output
OMAPDSS: TFP410: rename dvi -> tfp410
OMAPDSS: TFP410: rename dvi files to tfp410
OMAPDSS: Taal: move reset gpio handling to taal driver
OMAPDSS: DSI: implement generic DSI pin config
Merge branch 'archit/set-timing-work'
Merge branch 'for-l-o-3.5'
OMAPFB: fix parsing of vram parameter
OMAPDSS: TFP410: pdata rewrite
OMAPDSS: DSI: use dsi_get_dsidev_id(dsidev) instead of dsidev->id
OMAPDSS: clean up the omapdss platform data mess
OMAPDSS: remove return from platform_driver_unreg
OMAPDSS: use platform_driver_probe for core/dispc/dss
OMAPDSS: create custom pdevs for DSS omap_devices
OMAPDSS: create DPI & SDI devices
OMAPDSS: create DPI & SDI drivers
OMAPDSS: remove uses of dss_runtime_get/put
OMAPDSS: handle output-driver reg/unreg more dynamically
OMAPDSS: move the creation of debugfs files
OMAPDSS: use platform_driver_probe for dsi/hdmi/rfbi/venc/dpi/sdi
OMAPDSS: add __init & __exit
OMAPFB: add __init & __exit
OMAPDSS: change default_device handling
OMAPDSS: interface drivers register their panel devices
OMAPDSS: init omap_dss_devices internally
OMAPDSS: DSI: improve DSI module id handling
OMAPDSS: separate pdata based initialization
Merge branch 'dss-devtree-cleanup'
Merge branch 'omapdss-hdmi-audio'
OMAPDSS: use DSI_FIFO_BUG workaround only for manual update displays
OMAPDSS: DISPC: fix usage of dispc_ovl_set_accu_uv
OMAPDSS: remove compiler warnings when CONFIG_BUG=n
OMAPFB: remove compiler warnings when CONFIG_BUG=n
OMAPDSS: VRFB: remove compiler warnings when CONFIG_BUG=n
Documentation/arm/OMAP/DSS | 46 ++
arch/arm/mach-omap2/board-3430sdp.c | 38 +-
arch/arm/mach-omap2/board-4430sdp.c | 37 +-
arch/arm/mach-omap2/board-am3517evm.c | 25 +-
arch/arm/mach-omap2/board-cm-t35.c | 30 +-
arch/arm/mach-omap2/board-devkit8000.c | 30 +-
arch/arm/mach-omap2/board-igep0020.c | 32 +-
arch/arm/mach-omap2/board-omap3beagle.c | 37 +-
arch/arm/mach-omap2/board-omap3evm.c | 29 +-
arch/arm/mach-omap2/board-omap3stalker.c | 29 +-
arch/arm/mach-omap2/board-omap4panda.c | 39 +-
arch/arm/mach-omap2/board-overo.c | 25 +-
arch/arm/mach-omap2/display.c | 196 ++++-
drivers/video/omap/Kconfig | 8 -
drivers/video/omap2/displays/Kconfig | 8 +-
drivers/video/omap2/displays/Makefile | 2 +-
drivers/video/omap2/displays/panel-acx565akm.c | 7 -
drivers/video/omap2/displays/panel-generic-dpi.c | 107 ++-
drivers/video/omap2/displays/panel-n8x0.c | 8 -
drivers/video/omap2/displays/panel-taal.c | 108 +--
.../omap2/displays/{panel-dvi.c => panel-tfp410.c} | 186 ++---
.../video/omap2/displays/panel-tpo-td043mtea1.c | 22 +-
drivers/video/omap2/dss/Kconfig | 13 +-
drivers/video/omap2/dss/apply.c | 134 +++-
drivers/video/omap2/dss/core.c | 255 ++++---
drivers/video/omap2/dss/dispc.c | 747 ++++++++++++++------
drivers/video/omap2/dss/dispc.h | 72 ++
drivers/video/omap2/dss/display.c | 49 +-
drivers/video/omap2/dss/dpi.c | 75 +-
drivers/video/omap2/dss/dsi.c | 537 ++++++++------
drivers/video/omap2/dss/dss.c | 65 +-
drivers/video/omap2/dss/dss.h | 151 ++--
drivers/video/omap2/dss/dss_features.c | 30 +-
drivers/video/omap2/dss/dss_features.h | 5 +
drivers/video/omap2/dss/hdmi.c | 443 +++++-------
drivers/video/omap2/dss/hdmi_panel.c | 236 ++++++-
drivers/video/omap2/dss/manager.c | 19 +-
drivers/video/omap2/dss/overlay.c | 16 +-
drivers/video/omap2/dss/rfbi.c | 84 ++-
drivers/video/omap2/dss/sdi.c | 63 +-
drivers/video/omap2/dss/ti_hdmi.h | 32 +-
drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c | 480 ++++++++-----
drivers/video/omap2/dss/ti_hdmi_4xxx_ip.h | 161 +----
drivers/video/omap2/dss/venc.c | 133 +++-
drivers/video/omap2/omapfb/omapfb-ioctl.c | 17 +-
drivers/video/omap2/omapfb/omapfb-main.c | 12 +-
drivers/video/omap2/omapfb/omapfb.h | 1 +
drivers/video/omap2/vrfb.c | 4 +-
include/video/omap-panel-nokia-dsi.h | 3 +
.../{omap-panel-dvi.h => omap-panel-tfp410.h} | 18 +-
include/video/omapdss.h | 75 +-
51 files changed, 2983 insertions(+), 1996 deletions(-)
rename drivers/video/omap2/displays/{panel-dvi.c => panel-tfp410.c} (56%)
rename include/video/{omap-panel-dvi.h => omap-panel-tfp410.h} (63%)
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* [PATCH 0/2] OMAPDSS: write-through caching support for omapfb
From: Siarhei Siamashka @ 2012-05-22 19:54 UTC (permalink / raw)
To: linux-arm-kernel
This is a very simple few-liner patchset, which allows to optionally
enable write-through caching for OMAP DSS framebuffer. The problem with
the current writecombine cacheability attribute is that it only speeds
up writes. Uncached reads are slow, even though the use of NEON mitigates
this problem a bit.
Traditionally, xf86-video-fbdev DDX is using shadow framebuffer in the
system memory. Which contains a copy of the framebuffer data for the
purpose of providing fast read access to it when needed. Framebuffer
read access is required not so often, but it still gets used for
scrolling and moving windows around in Xorg server. And the users
perceive their linux desktop as rather sluggish when these operations
are not fast enough.
In the case of ARM hardware, framebuffer is typically physically
located in the main memory. And the processors still support
write-through cacheability attribute. According to ARM ARM, the writes
done to write-through cached memory inside the level of cache are
visible to all observers outside the level of cache without the need
of explicit cache maintenance (same rule as for non-cached memory).
So write-through cache is a perfect choice when only CPU is allowed
to modify the data in the framebuffer and everyone else (screen
refresh DMA) is only reading it. That is, assuming that write-through
cached memory provides good performance and there are no quirks.
As the framebuffer reads become fast, the need for shadow framebuffer
disappears.
And at least for ARM11 and Cortex-A8 processors, the performance of
write-through cache is really good. Cortex-A9 is another story, because
all pages marked as Write-Through are supposedly treated as Non-Cacheable:
http://infocenter.arm.com/help/topic/com.arm.doc.ddi0388h/CBBFDIJD.html
So OMAP4 is out of luck. But OMAP3 based hardware can have a nice
graphics performance boost. And OMAP3 actually needs it a lot more.
PS. The xf86-video-omapfb-0.1.1 driver does not even use shadow
framebuffer (ouch!). So its users, if any, should see an immediate
speedup.
Siarhei Siamashka (2):
ARM: pgtable: add pgprot_writethrough() macro
OMAPDSS: Optionally enable write-through cache for the framebuffer
Documentation/arm/OMAP/DSS | 10 ++++++++++
arch/arm/include/asm/pgtable.h | 3 +++
drivers/video/omap2/omapfb/omapfb-main.c | 7 ++++++-
3 files changed, 19 insertions(+), 1 deletions(-)
--
1.7.3.4
^ permalink raw reply
* [PATCH 1/2] ARM: pgtable: add pgprot_writethrough() macro
From: Siarhei Siamashka @ 2012-05-22 19:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1337716444-30139-1-git-send-email-siarhei.siamashka@gmail.com>
Needed for remapping pages with write-through cacheable
attribute. May be useful for framebuffers.
Signed-off-by: Siarhei Siamashka <siarhei.siamashka@gmail.com>
---
arch/arm/include/asm/pgtable.h | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/arch/arm/include/asm/pgtable.h b/arch/arm/include/asm/pgtable.h
index f66626d..04297fa 100644
--- a/arch/arm/include/asm/pgtable.h
+++ b/arch/arm/include/asm/pgtable.h
@@ -103,6 +103,9 @@ extern pgprot_t pgprot_kernel;
#define pgprot_stronglyordered(prot) \
__pgprot_modify(prot, L_PTE_MT_MASK, L_PTE_MT_UNCACHED)
+#define pgprot_writethrough(prot) \
+ __pgprot_modify(prot, L_PTE_MT_MASK, L_PTE_MT_WRITETHROUGH)
+
#ifdef CONFIG_ARM_DMA_MEM_BUFFERABLE
#define pgprot_dmacoherent(prot) \
__pgprot_modify(prot, L_PTE_MT_MASK, L_PTE_MT_BUFFERABLE | L_PTE_XN)
--
1.7.3.4
^ permalink raw reply related
* [PATCH 2/2] OMAPDSS: Optionally enable write-through cache for the framebuffer
From: Siarhei Siamashka @ 2012-05-22 19:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1337716444-30139-1-git-send-email-siarhei.siamashka@gmail.com>
Write-through cached framebuffer eliminates the need for using shadow
framebuffer in xf86-video-fbdev DDX. At the very least this reduces
memory footprint, but the performance is also the same or better
when moving windows or scrolling on ARM11 and Cortex-A8 hardware.
Benchmark with xf86-video-fbdev on IGEPv2 board (TI DM3730, 1GHz),
1280x1024 screen resolution and 32bpp desktop color depth:
$ x11perf -scroll500 -copywinwin500 -copypixpix500 \
-copypixwin500 -copywinpix500
-- omapfb.vram_cache=n, Option "ShadowFB" "true" in xorg.conf
10000 trep @ 3.4583 msec ( 289.0/sec): Scroll 500x500 pixels
6000 trep @ 4.3255 msec ( 231.0/sec): Copy 500x500 from window to window
8000 trep @ 3.2738 msec ( 305.0/sec): Copy 500x500 from pixmap to window
8000 trep @ 3.1707 msec ( 315.0/sec): Copy 500x500 from window to pixmap
8000 trep @ 3.4761 msec ( 288.0/sec): Copy 500x500 from pixmap to pixmap
-- omapfb.vram_cache=n, Option "ShadowFB" "false" in xorg.conf
5000 trep @ 5.2357 msec ( 191.0/sec): Scroll 500x500 pixels
1200 trep @ 21.0346 msec ( 47.5/sec): Copy 500x500 from window to window
8000 trep @ 3.1590 msec ( 317.0/sec): Copy 500x500 from pixmap to window
6000 trep @ 4.5062 msec ( 222.0/sec): Copy 500x500 from window to pixmap
8000 trep @ 3.4767 msec ( 288.0/sec): Copy 500x500 from pixmap to pixmap
-- omapfb.vram_cache=y, Option "ShadowFB" "true" in xorg.conf
10000 trep @ 3.4580 msec ( 289.0/sec): Scroll 500x500 pixels
6000 trep @ 4.3424 msec ( 230.0/sec): Copy 500x500 from window to window
8000 trep @ 3.2673 msec ( 306.0/sec): Copy 500x500 from pixmap to window
8000 trep @ 3.1626 msec ( 316.0/sec): Copy 500x500 from window to pixmap
8000 trep @ 3.4733 msec ( 288.0/sec): Copy 500x500 from pixmap to pixmap
-- omapfb.vram_cache=y, Option "ShadowFB" "false" in xorg.conf
10000 trep @ 3.4893 msec ( 287.0/sec): Scroll 500x500 pixels
8000 trep @ 4.0600 msec ( 246.0/sec): Copy 500x500 from window to window
8000 trep @ 3.1565 msec ( 317.0/sec): Copy 500x500 from pixmap to window
8000 trep @ 3.1373 msec ( 319.0/sec): Copy 500x500 from window to pixmap
8000 trep @ 3.4631 msec ( 289.0/sec): Copy 500x500 from pixmap to pixmap
Signed-off-by: Siarhei Siamashka <siarhei.siamashka@gmail.com>
---
Documentation/arm/OMAP/DSS | 10 ++++++++++
drivers/video/omap2/omapfb/omapfb-main.c | 7 ++++++-
2 files changed, 16 insertions(+), 1 deletions(-)
diff --git a/Documentation/arm/OMAP/DSS b/Documentation/arm/OMAP/DSS
index 888ae7b..4e41a15 100644
--- a/Documentation/arm/OMAP/DSS
+++ b/Documentation/arm/OMAP/DSS
@@ -293,6 +293,16 @@ omapfb.rotate=<angle>
omapfb.mirror=<y|n>
- Default mirror for all framebuffers. Only works with DMA rotation.
+omapfb.vram_cache=<y|n>
+ - Sets the framebuffer memory to be write-through cached. This may be
+ useful in the configurations where only CPU is allowed to write to
+ the framebuffer and eliminate the need for enabling shadow
+ framebuffer in Xorg DDX drivers such as xf86-video-fbdev and
+ xf86-video-omapfb. Enabling write-through cache is only useful
+ for ARM11 and Cortex-A8 processors. Cortex-A9 does not support
+ write-through cache well, see "Cortex-A9 behavior for Normal Memory
+ Cacheable memory regions" section in Cortex-A9 TRM for more details.
+
omapdss.def_disp=<display>
- Name of default display, to which all overlays will be connected.
Common examples are "lcd" or "tv".
diff --git a/drivers/video/omap2/omapfb/omapfb-main.c b/drivers/video/omap2/omapfb/omapfb-main.c
index b00db40..a684920 100644
--- a/drivers/video/omap2/omapfb/omapfb-main.c
+++ b/drivers/video/omap2/omapfb/omapfb-main.c
@@ -46,6 +46,7 @@ static char *def_vram;
static bool def_vrfb;
static int def_rotate;
static bool def_mirror;
+static bool def_vram_cache;
static bool auto_update;
static unsigned int auto_update_freq;
module_param(auto_update, bool, 0);
@@ -1123,7 +1124,10 @@ static int omapfb_mmap(struct fb_info *fbi, struct vm_area_struct *vma)
vma->vm_pgoff = off >> PAGE_SHIFT;
vma->vm_flags |= VM_IO | VM_RESERVED;
- vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot);
+ if (def_vram_cache)
+ vma->vm_page_prot = pgprot_writethrough(vma->vm_page_prot);
+ else
+ vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot);
vma->vm_ops = &mmap_user_ops;
vma->vm_private_data = rg;
if (io_remap_pfn_range(vma, vma->vm_start, off >> PAGE_SHIFT,
@@ -2493,6 +2497,7 @@ module_param_named(vram, def_vram, charp, 0);
module_param_named(rotate, def_rotate, int, 0);
module_param_named(vrfb, def_vrfb, bool, 0);
module_param_named(mirror, def_mirror, bool, 0);
+module_param_named(vram_cache, def_vram_cache, bool, 0444);
/* late_initcall to let panel/ctrl drivers loaded first.
* I guess better option would be a more dynamic approach,
--
1.7.3.4
^ permalink raw reply related
* Re: [PATCH 0/2] OMAPDSS: write-through caching support for omapfb
From: Siarhei Siamashka @ 2012-05-22 20:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1337716444-30139-1-git-send-email-siarhei.siamashka@gmail.com>
On Tue, May 22, 2012 at 10:54 PM, Siarhei Siamashka
<siarhei.siamashka@gmail.com> wrote:
> And at least for ARM11 and Cortex-A8 processors, the performance of
> write-through cache is really good. Cortex-A9 is another story, because
> all pages marked as Write-Through are supposedly treated as Non-Cacheable:
> http://infocenter.arm.com/help/topic/com.arm.doc.ddi0388h/CBBFDIJD.html
> So OMAP4 is out of luck.
I don't have Pandaboard ES, but still tried to experiment changing the
following line in the kernel sources to benchmark different types of
caching for the framebuffer on Origen board (Exynos 4210):
https://github.com/torvalds/linux/blob/v3.4/drivers/media/video/videobuf2-memops.c#L158
It was not a totally clean experiment, because 500x500 16bpp pixel
buffer is much smaller than 1MiB L2 cache and the performance numbers
may be a bit odd. Also I have not checked whether the same buffer may
be mapped with different cacheability attributes anywhere else (which
would be bad). But still it was interesting to see whether
write-through cache is of any use and whether it could serve as a
replacement for shadowfb.
Origen board, Exynos 4210, Cortex-A9 1.2GHz, 1920x1080 screen
resolution, 16bpp desktop color depth (I did not find any obvious way
how to change it to 32bpp yet):
$ x11perf -scroll500 -copywinwin500 -copypixpix500 \
-copypixwin500 -copywinpix500
-- pgprot_noncached + shadowfb
100000 trep @ 0.2708 msec ( 3690.0/sec): Scroll 500x500 pixels
40000 trep @ 0.7307 msec ( 1370.0/sec): Copy 500x500 from window to window
60000 trep @ 0.5471 msec ( 1830.0/sec): Copy 500x500 from pixmap to window
60000 trep @ 0.5822 msec ( 1720.0/sec): Copy 500x500 from window to pixmap
40000 trep @ 0.6584 msec ( 1520.0/sec): Copy 500x500 from pixmap to pixmap
-- pgprot_writecombine + shadowfb
100000 trep @ 0.2612 msec ( 3830.0/sec): Scroll 500x500 pixels
40000 trep @ 0.7058 msec ( 1420.0/sec): Copy 500x500 from window to window
60000 trep @ 0.5262 msec ( 1900.0/sec): Copy 500x500 from pixmap to window
60000 trep @ 0.5797 msec ( 1730.0/sec): Copy 500x500 from window to pixmap
40000 trep @ 0.6554 msec ( 1530.0/sec): Copy 500x500 from pixmap to pixmap
-- pgprot_writethrough + shadowfb
100000 trep @ 0.2609 msec ( 3830.0/sec): Scroll 500x500 pixels
40000 trep @ 0.7018 msec ( 1420.0/sec): Copy 500x500 from window to window
60000 trep @ 0.5260 msec ( 1900.0/sec): Copy 500x500 from pixmap to window
60000 trep @ 0.5758 msec ( 1740.0/sec): Copy 500x500 from window to pixmap
40000 trep @ 0.6569 msec ( 1520.0/sec): Copy 500x500 from pixmap to pixmap
-- pgprot_noncached
3500 trep @ 7.5972 msec ( 132.0/sec): Scroll 500x500 pixels
1800 trep @ 14.7146 msec ( 68.0/sec): Copy 500x500 from window to window
6000 trep @ 4.6501 msec ( 215.0/sec): Copy 500x500 from pixmap to window
8000 trep @ 3.3500 msec ( 299.0/sec): Copy 500x500 from window to pixmap
40000 trep @ 0.6546 msec ( 1530.0/sec): Copy 500x500 from pixmap to pixmap
-- pgprot_writecombine
10000 trep @ 2.9439 msec ( 340.0/sec): Scroll 500x500 pixels
6000 trep @ 5.7246 msec ( 175.0/sec): Copy 500x500 from window to window
60000 trep @ 0.4213 msec ( 2370.0/sec): Copy 500x500 from pixmap to window
12000 trep @ 2.2423 msec ( 446.0/sec): Copy 500x500 from window to pixmap
40000 trep @ 0.6648 msec ( 1500.0/sec): Copy 500x500 from pixmap to pixmap
-- pgprot_writethrough
40000 trep @ 0.7103 msec ( 1410.0/sec): Scroll 500x500 pixels
20000 trep @ 1.3024 msec ( 768.0/sec): Copy 500x500 from window to window
80000 trep @ 0.3933 msec ( 2540.0/sec): Copy 500x500 from pixmap to window
18000 trep @ 1.3967 msec ( 716.0/sec): Copy 500x500 from window to pixmap
40000 trep @ 0.6548 msec ( 1530.0/sec): Copy 500x500 from pixmap to pixmap
Without shadowfb, the performance of "writecombine" looks to be better
than "noncached". And "writethrough" is clearly the fastest. Still
even "writethrough" is no match for shadowfb on Cortex-A9 (unlike
ARM11 and Cortex-A8).
So is Cortex-A9 a lost cause? Maybe experimenting with page table
entries and tweaking inner/outer cacheability attributes could provide
something? From the first glance, it looks like read performance for
write-through cached memory is rather bad on Cortex-A9. But still
there is some speedup, so it does not seem to be treated as totally
non-cached. And at least PL310 L2 cache controller has some support
for "Cacheable write-through, allocate on read":
http://infocenter.arm.com/help/topic/com.arm.doc.ddi0246f/ch02s03s01.html
--
Best regards,
Siarhei Siamashka
^ permalink raw reply
* Re: [PATCH] backlight: initialize struct backlight_properties properly
From: Andrew Morton @ 2012-05-22 20:59 UTC (permalink / raw)
To: Corentin Chary
Cc: Arnd Bergmann, Mathieu Poirier, Tobias Schandinat, Mark Brown,
linux-kernel, dri-devel, platform-driver-x86, patches, Florian,
Dave, Richard Purdie, linux-fbdev, Lars-Peter, Airlie, Axel Lin,
linux-omap, Matthew Garrett
In-Reply-To: <1337585085-4891-1-git-send-email-corentin.chary@gmail.com>
On Mon, 21 May 2012 09:24:35 +0200
Corentin Chary <corentin.chary@gmail.com> wrote:
> In all these files, the .power field was never correctly initialized.
>
> ...
>
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -342,6 +342,7 @@ int intel_panel_setup_backlight(struct drm_device *dev)
> else
> return -ENODEV;
>
> + memset(&props, 0, sizeof(props));
This code is all still quite fragile. What happens if we add a new
field to backlight_properties? We need to go edit a zillion drivers?
There are two ways of fixing this:
a) write and use
void backlight_properties_init(struct backlight_properties *props,
int type, int max_brightness, etc);
or
b) edit all sites to do
struct backlight_properties bl_props = {
.type = BACKLIGHT_RAW,
.max_brighness = intel_panel_get_max_backlight(dev),
.power = FB_BLANK_UNBLANK,
};
they're largely equivalent - the struct will be zeroed out and
then certain fields will be initialised. a) is a bit better because
some future field may not want the all-zeroes pattern (eg, a spinlock).
But they're both better than what we have now!
^ permalink raw reply
* [PATCH] da8xx-fb: Rounding FB size to satisfy SGX buffer requirements
From: Aditya Nellutla @ 2012-05-23 6:18 UTC (permalink / raw)
To: linux-fbdev
In the real time use-case when SGX is used for rendering to FB buffers it has been
observed that, the available memory from framebuffer driver is not sufficient for
SGX under certain cases (like 16-bit WVGA resolution). SGX requires 2 swap buffers
with each of the buffers aligned to lcm(line_length, PAGE_SIZE).
Inorder to satisfy this requirement, we have two options,
- Increase number of FB buffers (LCD_NUM_BUFFERS) to 3. This is not
recommended as we end up wasting huge memory in most of the cases.
- Align FB buffers to lcm(line_length, PAGE_SIZE).This ensures framebuffer
size is increased to satisfy SGX requirements keeping alignment intact.
This patch makes sure that FB allocates buffers aligned to above formula.
Signed-off-by: Aditya Nellutla <aditya.n@ti.com>
---
drivers/video/da8xx-fb.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 47118c7..2f24c19 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -31,6 +31,7 @@
#include <linux/cpufreq.h>
#include <linux/console.h>
#include <linux/slab.h>
+#include <linux/lcm.h>
#include <video/da8xx-fb.h>
#include <asm/div64.h>
@@ -1114,6 +1115,7 @@ static int __devinit fb_probe(struct platform_device *device)
struct da8xx_fb_par *par;
resource_size_t len;
int ret, i;
+ unsigned long ulcm;
if (fb_pdata = NULL) {
dev_err(&device->dev, "Can not get platform data\n");
@@ -1209,7 +1211,8 @@ static int __devinit fb_probe(struct platform_device *device)
/* allocate frame buffer */
par->vram_size = lcdc_info->width * lcdc_info->height * lcd_cfg->bpp;
- par->vram_size = PAGE_ALIGN(par->vram_size/8);
+ ulcm = lcm((lcdc_info->width * lcd_cfg->bpp)/8, PAGE_SIZE);
+ par->vram_size = roundup(par->vram_size/8, ulcm);
par->vram_size = par->vram_size * LCD_NUM_BUFFERS;
par->vram_virt = dma_alloc_coherent(NULL,
--
1.7.0.4
^ permalink raw reply related
* Re: [PATCH 0/2] OMAPDSS: write-through caching support for omapfb
From: Tomi Valkeinen @ 2012-05-24 7:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1337716444-30139-1-git-send-email-siarhei.siamashka@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 4056 bytes --]
On Tue, 2012-05-22 at 22:54 +0300, Siarhei Siamashka wrote:
> This is a very simple few-liner patchset, which allows to optionally
> enable write-through caching for OMAP DSS framebuffer. The problem with
> the current writecombine cacheability attribute is that it only speeds
> up writes. Uncached reads are slow, even though the use of NEON mitigates
> this problem a bit.
>
> Traditionally, xf86-video-fbdev DDX is using shadow framebuffer in the
> system memory. Which contains a copy of the framebuffer data for the
> purpose of providing fast read access to it when needed. Framebuffer
> read access is required not so often, but it still gets used for
> scrolling and moving windows around in Xorg server. And the users
> perceive their linux desktop as rather sluggish when these operations
> are not fast enough.
>
> In the case of ARM hardware, framebuffer is typically physically
> located in the main memory. And the processors still support
> write-through cacheability attribute. According to ARM ARM, the writes
> done to write-through cached memory inside the level of cache are
> visible to all observers outside the level of cache without the need
> of explicit cache maintenance (same rule as for non-cached memory).
> So write-through cache is a perfect choice when only CPU is allowed
> to modify the data in the framebuffer and everyone else (screen
> refresh DMA) is only reading it. That is, assuming that write-through
> cached memory provides good performance and there are no quirks.
> As the framebuffer reads become fast, the need for shadow framebuffer
> disappears.
I ran my own fb perf test on omap3 overo board ("perf" test in
https://gitorious.org/linux-omap-dss2/omapfb-tests) :
vram_cache=n:
sequential_horiz_singlepixel_read: 25198080 pix, 4955475 us, 5084897 pix/s
sequential_horiz_singlepixel_write: 434634240 pix, 4081146 us, 106498086 pix/s
sequential_vert_singlepixel_read: 20106240 pix, 4970611 us, 4045023 pix/s
sequential_vert_singlepixel_write: 98572800 pix, 4985748 us, 19770915 pix/s
sequential_line_read: 40734720 pix, 4977906 us, 8183103 pix/s
sequential_line_write: 1058580480 pix, 5024628 us, 210678378 pix/s
nonsequential_singlepixel_write: 17625600 pix, 4992828 us, 3530183 pix/s
nonsequential_singlepixel_read: 9661440 pix, 4952973 us, 1950634 pix/s
vram_cache=y:
sequential_horiz_singlepixel_read: 270389760 pix, 4994154 us, 54141253 pix/s
sequential_horiz_singlepixel_write: 473149440 pix, 3932801 us, 120308512 pix/s
sequential_vert_singlepixel_read: 18147840 pix, 4976226 us, 3646908 pix/s
sequential_vert_singlepixel_write: 100661760 pix, 4993164 us, 20159914 pix/s
sequential_line_read: 285143040 pix, 4917267 us, 57988114 pix/s
sequential_line_write: 876710400 pix, 5012146 us, 174917171 pix/s
nonsequential_singlepixel_write: 17625600 pix, 4977967 us, 3540722 pix/s
nonsequential_singlepixel_read: 9661440 pix, 4944885 us, 1953825 pix/s
These also show quite a bit of improvement in some read cases.
Interestingly some of the write cases are also faster.
Reading pixels vertically is slower with vram_cache. I guess this is
because the cache causes some overhead, and we always miss the cache so
the caching is just wasted time.
I would've also presumed the difference in sequential_line_write would
be bigger. write-through is effectively no-cache for writes, right?
If the user of the fb just writes to the fb and vram_cache=y, it means
that the cache is filled with pixel data that is never used, thus
lowering the performance of all other programs?
I have to say I don't know much of the cpu caches, but the read speed
improvements are very big, so I think this is definitely interesting
patch. So if you get the first patch accepted I see no problem with
adding this to omapfb as an optional feature.
However, "vram_cache" is not a very good name for the option.
"vram_writethrough", or something?
Did you test this with VRFB (omap3) or TILER (omap4)? I wonder how those
are affected.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* [PATCH 1/3] video: exynos_dp: use devm_ functions
From: Jingoo Han @ 2012-05-25 7:20 UTC (permalink / raw)
To: linux-fbdev
The devm_ functions allocate memory that is released when a driver
detaches. This makes the code smaller and a bit simpler.
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
drivers/video/exynos/exynos_dp_core.c | 49 ++++++++-------------------------
drivers/video/exynos/exynos_dp_core.h | 1 -
2 files changed, 12 insertions(+), 38 deletions(-)
diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
index ef4d1ab..aabd214 100644
--- a/drivers/video/exynos/exynos_dp_core.c
+++ b/drivers/video/exynos/exynos_dp_core.c
@@ -858,7 +858,8 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
return -EINVAL;
}
- dp = kzalloc(sizeof(struct exynos_dp_device), GFP_KERNEL);
+ dp = devm_kzalloc(&pdev->dev, sizeof(struct exynos_dp_device),
+ GFP_KERNEL);
if (!dp) {
dev_err(&pdev->dev, "no memory for device data\n");
return -ENOMEM;
@@ -869,8 +870,7 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
dp->clock = clk_get(&pdev->dev, "dp");
if (IS_ERR(dp->clock)) {
dev_err(&pdev->dev, "failed to get clock\n");
- ret = PTR_ERR(dp->clock);
- goto err_dp;
+ return PTR_ERR(dp->clock);
}
clk_enable(dp->clock);
@@ -882,35 +882,25 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
goto err_clock;
}
- res = request_mem_region(res->start, resource_size(res),
- dev_name(&pdev->dev));
- if (!res) {
- dev_err(&pdev->dev, "failed to request registers region\n");
- ret = -EINVAL;
- goto err_clock;
- }
-
- dp->res = res;
-
- dp->reg_base = ioremap(res->start, resource_size(res));
+ dp->reg_base = devm_request_and_ioremap(&pdev->dev, res);
if (!dp->reg_base) {
dev_err(&pdev->dev, "failed to ioremap\n");
ret = -ENOMEM;
- goto err_req_region;
+ goto err_clock;
}
dp->irq = platform_get_irq(pdev, 0);
if (!dp->irq) {
dev_err(&pdev->dev, "failed to get irq\n");
ret = -ENODEV;
- goto err_ioremap;
+ goto err_clock;
}
- ret = request_irq(dp->irq, exynos_dp_irq_handler, 0,
- "exynos-dp", dp);
+ ret = devm_request_irq(&pdev->dev, dp->irq, exynos_dp_irq_handler, 0,
+ "exynos-dp", dp);
if (ret) {
dev_err(&pdev->dev, "failed to request irq\n");
- goto err_ioremap;
+ goto err_clock;
}
dp->video_info = pdata->video_info;
@@ -922,7 +912,7 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
ret = exynos_dp_detect_hpd(dp);
if (ret) {
dev_err(&pdev->dev, "unable to detect hpd\n");
- goto err_irq;
+ goto err_clock;
}
exynos_dp_handle_edid(dp);
@@ -931,7 +921,7 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
dp->video_info->link_rate);
if (ret) {
dev_err(&pdev->dev, "unable to do link train\n");
- goto err_irq;
+ goto err_clock;
}
exynos_dp_enable_scramble(dp, 1);
@@ -945,23 +935,15 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
ret = exynos_dp_config_video(dp, dp->video_info);
if (ret) {
dev_err(&pdev->dev, "unable to config video\n");
- goto err_irq;
+ goto err_clock;
}
platform_set_drvdata(pdev, dp);
return 0;
-err_irq:
- free_irq(dp->irq, dp);
-err_ioremap:
- iounmap(dp->reg_base);
-err_req_region:
- release_mem_region(res->start, resource_size(res));
err_clock:
clk_put(dp->clock);
-err_dp:
- kfree(dp);
return ret;
}
@@ -974,16 +956,9 @@ static int __devexit exynos_dp_remove(struct platform_device *pdev)
if (pdata && pdata->phy_exit)
pdata->phy_exit();
- free_irq(dp->irq, dp);
- iounmap(dp->reg_base);
-
clk_disable(dp->clock);
clk_put(dp->clock);
- release_mem_region(dp->res->start, resource_size(dp->res));
-
- kfree(dp);
-
return 0;
}
diff --git a/drivers/video/exynos/exynos_dp_core.h b/drivers/video/exynos/exynos_dp_core.h
index 519c3a6..8f596b9 100644
--- a/drivers/video/exynos/exynos_dp_core.h
+++ b/drivers/video/exynos/exynos_dp_core.h
@@ -26,7 +26,6 @@ struct link_train {
struct exynos_dp_device {
struct device *dev;
- struct resource *res;
struct clk *clock;
unsigned int irq;
void __iomem *reg_base;
--
1.7.1
^ permalink raw reply related
* [PATCH 2/3] video: exynos_dp: move sw reset prioir to enabling sw defined function
From: Jingoo Han @ 2012-05-25 7:21 UTC (permalink / raw)
To: linux-fbdev
The sw reset should be called prioir to enabling sw defined function,
according to datasheet.
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
drivers/video/exynos/exynos_dp_core.c | 2 ++
drivers/video/exynos/exynos_dp_core.h | 1 +
drivers/video/exynos/exynos_dp_reg.c | 7 +++++--
3 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
index aabd214..b446f7e 100644
--- a/drivers/video/exynos/exynos_dp_core.c
+++ b/drivers/video/exynos/exynos_dp_core.c
@@ -27,6 +27,8 @@ static int exynos_dp_init_dp(struct exynos_dp_device *dp)
{
exynos_dp_reset(dp);
+ exynos_dp_swreset(dp);
+
/* SW defined function Normal operation */
exynos_dp_enable_sw_function(dp);
diff --git a/drivers/video/exynos/exynos_dp_core.h b/drivers/video/exynos/exynos_dp_core.h
index 8f596b9..1e0f998 100644
--- a/drivers/video/exynos/exynos_dp_core.h
+++ b/drivers/video/exynos/exynos_dp_core.h
@@ -41,6 +41,7 @@ void exynos_dp_lane_swap(struct exynos_dp_device *dp, bool enable);
void exynos_dp_init_analog_param(struct exynos_dp_device *dp);
void exynos_dp_init_interrupt(struct exynos_dp_device *dp);
void exynos_dp_reset(struct exynos_dp_device *dp);
+void exynos_dp_swreset(struct exynos_dp_device *dp);
void exynos_dp_config_interrupt(struct exynos_dp_device *dp);
u32 exynos_dp_get_pll_lock_status(struct exynos_dp_device *dp);
void exynos_dp_set_pll_power_down(struct exynos_dp_device *dp, bool enable);
diff --git a/drivers/video/exynos/exynos_dp_reg.c b/drivers/video/exynos/exynos_dp_reg.c
index 99bafb8..6ce76d5 100644
--- a/drivers/video/exynos/exynos_dp_reg.c
+++ b/drivers/video/exynos/exynos_dp_reg.c
@@ -109,8 +109,6 @@ void exynos_dp_reset(struct exynos_dp_device *dp)
{
u32 reg;
- writel(RESET_DP_TX, dp->reg_base + EXYNOS_DP_TX_SW_RESET);
-
exynos_dp_stop_video(dp);
exynos_dp_enable_video_mute(dp, 0);
@@ -155,6 +153,11 @@ void exynos_dp_reset(struct exynos_dp_device *dp)
exynos_dp_init_interrupt(dp);
}
+void exynos_dp_swreset(struct exynos_dp_device *dp)
+{
+ writel(RESET_DP_TX, dp->reg_base + EXYNOS_DP_TX_SW_RESET);
+}
+
void exynos_dp_config_interrupt(struct exynos_dp_device *dp)
{
u32 reg;
--
1.7.1
^ permalink raw reply related
* [PATCH 3/3] video: exynos_dp: reduce delay time when configuring video setting
From: Jingoo Han @ 2012-05-25 7:21 UTC (permalink / raw)
To: linux-fbdev
This patch reduces delay time when configuring video setting,
which is helpful to reduce wakeup time during resume.
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
drivers/video/exynos/exynos_dp_core.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
index b446f7e..a36b2d2 100644
--- a/drivers/video/exynos/exynos_dp_core.c
+++ b/drivers/video/exynos/exynos_dp_core.c
@@ -770,7 +770,7 @@ static int exynos_dp_config_video(struct exynos_dp_device *dp,
return -ETIMEDOUT;
}
- mdelay(100);
+ udelay(1);
}
/* Set to use the register calculated M/N video */
@@ -804,7 +804,7 @@ static int exynos_dp_config_video(struct exynos_dp_device *dp,
return -ETIMEDOUT;
}
- mdelay(100);
+ mdelay(1);
}
if (retval != 0)
--
1.7.1
^ permalink raw reply related
* Re: [PATCH 0/2] OMAPDSS: write-through caching support for omapfb
From: Siarhei Siamashka @ 2012-05-25 9:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1337845417.2909.31.camel@deskari>
On Thu, May 24, 2012 at 10:43 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On Tue, 2012-05-22 at 22:54 +0300, Siarhei Siamashka wrote:
> I ran my own fb perf test on omap3 overo board ("perf" test in
> https://gitorious.org/linux-omap-dss2/omapfb-tests) :
>
> vram_cache=n:
>
> sequential_horiz_singlepixel_read: 25198080 pix, 4955475 us, 5084897 pix/s
> sequential_horiz_singlepixel_write: 434634240 pix, 4081146 us, 106498086 pix/s
> sequential_vert_singlepixel_read: 20106240 pix, 4970611 us, 4045023 pix/s
> sequential_vert_singlepixel_write: 98572800 pix, 4985748 us, 19770915 pix/s
> sequential_line_read: 40734720 pix, 4977906 us, 8183103 pix/s
> sequential_line_write: 1058580480 pix, 5024628 us, 210678378 pix/s
> nonsequential_singlepixel_write: 17625600 pix, 4992828 us, 3530183 pix/s
> nonsequential_singlepixel_read: 9661440 pix, 4952973 us, 1950634 pix/s
>
> vram_cache=y:
>
> sequential_horiz_singlepixel_read: 270389760 pix, 4994154 us, 54141253 pix/s
> sequential_horiz_singlepixel_write: 473149440 pix, 3932801 us, 120308512 pix/s
> sequential_vert_singlepixel_read: 18147840 pix, 4976226 us, 3646908 pix/s
> sequential_vert_singlepixel_write: 100661760 pix, 4993164 us, 20159914 pix/s
> sequential_line_read: 285143040 pix, 4917267 us, 57988114 pix/s
> sequential_line_write: 876710400 pix, 5012146 us, 174917171 pix/s
> nonsequential_singlepixel_write: 17625600 pix, 4977967 us, 3540722 pix/s
> nonsequential_singlepixel_read: 9661440 pix, 4944885 us, 1953825 pix/s
>
> These also show quite a bit of improvement in some read cases.
> Interestingly some of the write cases are also faster.
>
> Reading pixels vertically is slower with vram_cache. I guess this is
> because the cache causes some overhead, and we always miss the cache so
> the caching is just wasted time.
On a positive side, nobody is normally accessing memory in this way.
It is a well known performance anti-pattern.
> I would've also presumed the difference in sequential_line_write would
> be bigger. write-through is effectively no-cache for writes, right?
Write-through cache is still using write combining buffer for memory
writes. So I would actually expect the performance to be the same.
> If the user of the fb just writes to the fb and vram_cache=y, it means
> that the cache is filled with pixel data that is never used, thus
> lowering the performance of all other programs?
This is true only for write-allocate. We want the framebuffer to be
cacheable as write-through, allocate on read, no allocate on write.
Sure, when we are reading from the cached framebuffer, some useful
data may be evicted from cache. But if we are not caching the
framebuffer, any readers suffer from a huge performance penalty.
That's the reason why shadow framebuffer (a poor man's software
workaround) is implemented in Xorg server. And if we use a shadow
framebuffer (in normal write-back cached memory), we already have the
same or worse cache eviction problems when compared to the cached
framebuffer. I could not find any use case or benchmark where shadow
framebuffer would perform better than write-through cached framebuffer
on OMAP3 hardware.
Maybe a bit more details about the shadow framebuffer would be useful.
That's just one level above the framebuffer in the graphics stack for
linux desktop. And many of the performance issues happen exactly on
the boundary between different pieces of software when they are
developed independently. Knowing how the framebuffer is used in the
real world applications (and I assume X server is one of them) may
provide some insights about how to improve the framebuffer on the
kernel side.
Here the shadow framebuffer is initialized:
http://cgit.freedesktop.org/xorg/xserver/tree/miext/shadow/shadow.c?id=xorg-server-1.12.1#n136
And it also enables damage extension to track all the drawing
operations performed with the shadow buffer in order to copy the
updated areas to the real framebuffer from time to time. The update
itself happens here:
http://cgit.freedesktop.org/xorg/xserver/tree/miext/shadow/shpacked.c?id=xorg-server-1.12.1#n43
With the damage extension active, we go through damageCopyArea:
http://cgit.freedesktop.org/xorg/xserver/tree/miext/damage/damage.c?id=xorg-server-1.12.1#n801
before reaching fbCopyArea:
http://cgit.freedesktop.org/xorg/xserver/tree/fb/fbcopy.c?id=xorg-server-1.12.1#n265
While running x11perf test and doing copying/scrolling tests, the
function calls inside of X server look more or less like this:
62. [00:29:27.779] fbCopyArea()
63. [00:29:27.779] DamageReportDamage()
64. [00:29:27.783] fbCopyArea()
65. [00:29:27.783] DamageReportDamage()
66. [00:29:27.787] fbCopyArea()
67. [00:29:27.792] shadowUpdatePacked()
68. [00:29:27.792] DamageReportDamage()
69. [00:29:27.795] fbCopyArea()
70. [00:29:27.795] DamageReportDamage()
71. [00:29:27.799] fbCopyArea()
72. [00:29:27.800] DamageReportDamage()
73. [00:29:27.803] fbCopyArea()
74. [00:29:27.803] DamageReportDamage()
75. [00:29:27.807] fbCopyArea()
76. [00:29:27.807] DamageReportDamage()
77. [00:29:27.811] fbCopyArea()
78. [00:29:27.811] DamageReportDamage()
79. [00:29:27.815] fbCopyArea()
80. [00:29:27.820] shadowUpdatePacked()
As can be seen in the log above, shadowUpdatePacked() is used much
less frequently than fbCopyArea(). Which means that shadow framebuffer
is cheating a bit by accumulating damage and updating the real
framebuffer much less frequently.
The write-through cached framebuffer beats the shadow framebuffer in every way:
- It needs less RAM
- No need for damage tracking overhead (important for small drawing operations)
- No screen updates are skipped, which means smoother animation
> I have to say I don't know much of the cpu caches, but the read speed
> improvements are very big, so I think this is definitely interesting
> patch.
Yes, it is definitely providing a significant performance improvement
for software rendering in Xorg server. But there is unfortunately no
free lunch. Using write-through cache means that if anything other
than CPU is writing to the framebuffer, then it must invalidate CPU
cache for this area. And this means that it makes sense to review how
the SGX integration is done and fix it if needed. I tried some tests
with X11WSEGL (a binary blob provided in GFX SDK for X11 integration)
and it did not seem to exhibit any obvious screen corruption issues,
but without having the sources it's hard to say for sure. I would
assume that the ideal setup for OMAP3 would be to use GFX plane
exclusively by the CPU for 2D graphics, and render SGX 3D graphics to
one of the VID planes. In this case DISPC can do the compositing and
CPU cache invalidate operations become unnecessary.
There is also DSP, ISP and maybe some other hardware blocks, but these
can be handled on case by case basis. My primary interest is a little
personal hobby project to get linux desktop running with acceptable
performance.
> So if you get the first patch accepted I see no problem with
> adding this to omapfb as an optional feature.
Yes, a review from ARM memory management subsystem experts is definitely needed.
> However, "vram_cache" is not a very good name for the option.
> "vram_writethrough", or something?
Still having "cache" in the name would be useful. Just in order to
imply that there might be coherency issues to consider. By the way,
vesafb actually uses some numeric codes for different types of caching
in mtrr:n option:
https://github.com/torvalds/linux/blob/v3.4/Documentation/fb/vesafb.txt#L147
Write-through caching is bad on Cortex-A9 as promised in TRM and as
confirmed by tests. Still for Cortex-A9 it *might* be interesting to
experiment with enabling write-back cache for the framebuffer, but
instead of using shadow framebuffer just do CPU cache flushes based on
all the same damage tracking borrowed from shadow framebuffer code.
This even might be somehow related to the "manual" update mode :)
Except that we can't change framebuffer caching attributes at runtime.
> Did you test this with VRFB (omap3) or TILER (omap4)? I wonder how those
> are affected.
That's a good point. I tried VRFB on OMAP3 1GHz and got the following
results with x11perf (shadow framebuffer disabled in
xf86-video-fbdev):
------------------ rotate 90, write-through cached
3500 trep @ 8.0242 msec ( 125.0/sec): Scroll 500x500 pixels
4000 trep @ 8.7027 msec ( 115.0/sec): Copy 500x500 from window to window
6000 trep @ 5.4885 msec ( 182.0/sec): Copy 500x500 from pixmap to window
6000 trep @ 4.9806 msec ( 201.0/sec): Copy 500x500 from window to pixmap
8000 trep @ 3.4231 msec ( 292.0/sec): Copy 500x500 from pixmap to pixmap
------------------ rotate 90, non-cached writecombining
3000 trep @ 8.9732 msec ( 111.0/sec): Scroll 500x500 pixels
1000 trep @ 26.3218 msec ( 38.0/sec): Copy 500x500 from window to window
6000 trep @ 5.5002 msec ( 182.0/sec): Copy 500x500 from pixmap to window
6000 trep @ 6.2368 msec ( 160.0/sec): Copy 500x500 from window to pixmap
8000 trep @ 3.4219 msec ( 292.0/sec): Copy 500x500 from pixmap to pixmap
------------------ rotate 180, write-through cached
10000 trep @ 3.5219 msec ( 284.0/sec): Scroll 500x500 pixels
6000 trep @ 4.8829 msec ( 205.0/sec): Copy 500x500 from window to window
8000 trep @ 3.4772 msec ( 288.0/sec): Copy 500x500 from pixmap to window
8000 trep @ 3.2554 msec ( 307.0/sec): Copy 500x500 from window to pixmap
8000 trep @ 3.4196 msec ( 292.0/sec): Copy 500x500 from pixmap to pixmap
------------------ rotate 180, non-cached writecombining
10000 trep @ 4.5777 msec ( 218.0/sec): Scroll 500x500 pixels
1000 trep @ 24.9100 msec ( 40.1/sec): Copy 500x500 from window to window
8000 trep @ 3.4763 msec ( 288.0/sec): Copy 500x500 from pixmap to window
6000 trep @ 4.8676 msec ( 205.0/sec): Copy 500x500 from window to pixmap
8000 trep @ 3.4205 msec ( 292.0/sec): Copy 500x500 from pixmap to pixmap
The 90 degrees rotation significantly reduces performance. Shadow
framebuffer is "faster", because it is skipping some work. But
write-through caching is at least not worse than the default
writecombine.
Regarding OMAP4. I only have an old pre-production Pandaboard EA1,
which only runs memory at half speed. It is useless for any
benchmarks.
--
Best regards,
Siarhei Siamashka
^ permalink raw reply
* (no subject)
From: robothroli company @ 2012-05-25 13:45 UTC (permalink / raw)
i am robothroli, Purchase manager from roli Merchant Ltd. We are
Import/export Company based in taiwan. We are interested in purchasing
your product and I would like to make an inquiry. Please inform me on:
Sample availability and price
Minimum order quantity
FOB Prices
Sincerely
Purchase Manager
robothroli
^ permalink raw reply
* [PATCH] video: bfin_adv7393fb: Fix cleanup code
From: Emil Goode @ 2012-05-28 14:40 UTC (permalink / raw)
To: FlorianSchandinat; +Cc: linux-fbdev, linux-kernel, kernel-janitors, Emil Goode
This patch fixes the cleanup code of the bfin_adv7393_fb_probe
function by changing the order in which cleanup is performed
and by adding one label.
Signed-off-by: Emil Goode <emilgoode@gmail.com>
---
drivers/video/bfin_adv7393fb.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/drivers/video/bfin_adv7393fb.c b/drivers/video/bfin_adv7393fb.c
index 1a268a2..ddbd031 100644
--- a/drivers/video/bfin_adv7393fb.c
+++ b/drivers/video/bfin_adv7393fb.c
@@ -414,7 +414,7 @@ static int __devinit bfin_adv7393_fb_probe(struct i2c_client *client,
if (ret) {
dev_err(&client->dev, "PPI0_FS3 GPIO request failed\n");
ret = -EBUSY;
- goto out_8;
+ goto out_9;
}
}
@@ -528,15 +528,18 @@ static int __devinit bfin_adv7393_fb_probe(struct i2c_client *client,
out_3:
free_dma(CH_PPI);
out_4:
- dma_free_coherent(NULL, fbdev->fb_len, fbdev->fb_mem,
- fbdev->dma_handle);
- out_5:
fb_dealloc_cmap(&fbdev->info.cmap);
- out_6:
+ out_5:
kfree(fbdev->info.pseudo_palette);
+ out_6:
+ dma_free_coherent(NULL, fbdev->fb_len, fbdev->fb_mem,
+ fbdev->dma_handle);
out_7:
peripheral_free_list(ppi_pins);
out_8:
+ if (ANOMALY_05000400)
+ gpio_free(P_IDENT(P_PPI0_FS3));
+ out_9:
kfree(fbdev);
return ret;
--
1.7.10
^ permalink raw reply related
* Re: [PATCH] video: bfin_adv7393fb: Fix cleanup code
From: Dan Carpenter @ 2012-05-28 15:26 UTC (permalink / raw)
To: Emil Goode; +Cc: FlorianSchandinat, linux-fbdev, linux-kernel, kernel-janitors
In-Reply-To: <1338216019-1288-1-git-send-email-emilgoode@gmail.com>
On Mon, May 28, 2012 at 04:40:19PM +0200, Emil Goode wrote:
> This patch fixes the cleanup code of the bfin_adv7393_fb_probe
> function by changing the order in which cleanup is performed
> and by adding one label.
>
Could you list the actual bugs which were fixed:
1) The resources were not freed in the order that we allocated them
so we call dma_free_coherent() before it was allocated.
2) The labels weren't in the right place which also meant that we
freed resources that weren't allocated.
3) We should free gpio_free(P_IDENT(P_PPI0_FS3)) before returning.
The new gpio_free() is more important than the added label.
Since you're changing all these, you may as well fix the label names
as well. The out_9 style labels suck. The right way is to give
them meaningful labels instead of GW-BASIC style labels.
Labels should be named after what happens when you arrive. Some
people name them after the start of the journey but that's a
mistake. It's just like the town of Chicago is still called Chicago
even though you might goto Chicago starting from Boston. Like this:
err_ppi:
free_dma(CH_PPI);
fb_dealloc_cmap(&fbdev->info.cmap);
err_palette:
kfree(fbdev->info.pseudo_palette);
regards,
dan carpenter
^ permalink raw reply
* Re: [PATCH] video: bfin_adv7393fb: Fix cleanup code
From: Emil Goode @ 2012-05-28 15:34 UTC (permalink / raw)
To: Dan Carpenter
Cc: FlorianSchandinat, linux-fbdev, linux-kernel, kernel-janitors
In-Reply-To: <20120528152601.GE4637@mwanda>
Thanks Dan, I'll resend with these modifications.
Best regards,
Emil Goode
On Mon, 2012-05-28 at 18:26 +0300, Dan Carpenter wrote:
> On Mon, May 28, 2012 at 04:40:19PM +0200, Emil Goode wrote:
> > This patch fixes the cleanup code of the bfin_adv7393_fb_probe
> > function by changing the order in which cleanup is performed
> > and by adding one label.
> >
>
> Could you list the actual bugs which were fixed:
>
> 1) The resources were not freed in the order that we allocated them
> so we call dma_free_coherent() before it was allocated.
> 2) The labels weren't in the right place which also meant that we
> freed resources that weren't allocated.
> 3) We should free gpio_free(P_IDENT(P_PPI0_FS3)) before returning.
>
> The new gpio_free() is more important than the added label.
>
> Since you're changing all these, you may as well fix the label names
> as well. The out_9 style labels suck. The right way is to give
> them meaningful labels instead of GW-BASIC style labels.
>
> Labels should be named after what happens when you arrive. Some
> people name them after the start of the journey but that's a
> mistake. It's just like the town of Chicago is still called Chicago
> even though you might goto Chicago starting from Boston. Like this:
>
> err_ppi:
> free_dma(CH_PPI);
> fb_dealloc_cmap(&fbdev->info.cmap);
> err_palette:
> kfree(fbdev->info.pseudo_palette);
>
> regards,
> dan carpenter
^ permalink raw reply
* [PATCH v2] video: bfin_adv7393fb: Fix cleanup code
From: Emil Goode @ 2012-05-28 16:54 UTC (permalink / raw)
To: FlorianSchandinat; +Cc: linux-fbdev, linux-kernel, kernel-janitors, Emil Goode
This patch fixes the cleanup code of the bfin_adv7393_fb_probe
function.
1) The resources were not freed in the order that we allocated them
so we call dma_free_coherent() before it was allocated.
2) The labels weren't in the right place which also meant that we
freed resources that weren't allocated.
3) We should free gpio_free(P_IDENT(P_PPI0_FS3)) before returning.
4) Lets change the label names into something more meaningful.
Signed-off-by: Emil Goode <emilgoode@gmail.com>
---
v2: Changed the label names as well.
drivers/video/bfin_adv7393fb.c | 43 +++++++++++++++++++++-------------------
1 file changed, 23 insertions(+), 20 deletions(-)
diff --git a/drivers/video/bfin_adv7393fb.c b/drivers/video/bfin_adv7393fb.c
index 1a268a2..33ea874 100644
--- a/drivers/video/bfin_adv7393fb.c
+++ b/drivers/video/bfin_adv7393fb.c
@@ -414,14 +414,14 @@ static int __devinit bfin_adv7393_fb_probe(struct i2c_client *client,
if (ret) {
dev_err(&client->dev, "PPI0_FS3 GPIO request failed\n");
ret = -EBUSY;
- goto out_8;
+ goto free_fbdev;
}
}
if (peripheral_request_list(ppi_pins, DRIVER_NAME)) {
dev_err(&client->dev, "requesting PPI peripheral failed\n");
ret = -EFAULT;
- goto out_8;
+ goto free_gpio;
}
fbdev->fb_mem @@ -432,7 +432,7 @@ static int __devinit bfin_adv7393_fb_probe(struct i2c_client *client,
dev_err(&client->dev, "couldn't allocate dma buffer (%d bytes)\n",
(u32) fbdev->fb_len);
ret = -ENOMEM;
- goto out_7;
+ goto free_ppi_pins;
}
fbdev->info.screen_base = (void *)fbdev->fb_mem;
@@ -464,27 +464,27 @@ static int __devinit bfin_adv7393_fb_probe(struct i2c_client *client,
if (!fbdev->info.pseudo_palette) {
dev_err(&client->dev, "failed to allocate pseudo_palette\n");
ret = -ENOMEM;
- goto out_6;
+ goto free_fb_mem;
}
if (fb_alloc_cmap(&fbdev->info.cmap, BFIN_LCD_NBR_PALETTE_ENTRIES, 0) < 0) {
dev_err(&client->dev, "failed to allocate colormap (%d entries)\n",
BFIN_LCD_NBR_PALETTE_ENTRIES);
ret = -EFAULT;
- goto out_5;
+ goto free_palette;
}
if (request_dma(CH_PPI, "BF5xx_PPI_DMA") < 0) {
dev_err(&client->dev, "unable to request PPI DMA\n");
ret = -EFAULT;
- goto out_4;
+ goto free_cmap;
}
if (request_irq(IRQ_PPI_ERROR, ppi_irq_error, 0,
"PPI ERROR", fbdev) < 0) {
dev_err(&client->dev, "unable to request PPI ERROR IRQ\n");
ret = -EFAULT;
- goto out_3;
+ goto free_ch_ppi;
}
fbdev->open = 0;
@@ -494,14 +494,14 @@ static int __devinit bfin_adv7393_fb_probe(struct i2c_client *client,
if (ret) {
dev_err(&client->dev, "i2c attach: init error\n");
- goto out_1;
+ goto free_irq_ppi;
}
if (register_framebuffer(&fbdev->info) < 0) {
dev_err(&client->dev, "unable to register framebuffer\n");
ret = -EFAULT;
- goto out_1;
+ goto free_irq_ppi;
}
dev_info(&client->dev, "fb%d: %s frame buffer device\n",
@@ -512,7 +512,7 @@ static int __devinit bfin_adv7393_fb_probe(struct i2c_client *client,
if (!entry) {
dev_err(&client->dev, "unable to create /proc entry\n");
ret = -EFAULT;
- goto out_0;
+ goto free_fb;
}
entry->read_proc = adv7393_read_proc;
@@ -521,22 +521,25 @@ static int __devinit bfin_adv7393_fb_probe(struct i2c_client *client,
return 0;
- out_0:
+free_fb:
unregister_framebuffer(&fbdev->info);
- out_1:
+free_irq_ppi:
free_irq(IRQ_PPI_ERROR, fbdev);
- out_3:
+free_ch_ppi:
free_dma(CH_PPI);
- out_4:
- dma_free_coherent(NULL, fbdev->fb_len, fbdev->fb_mem,
- fbdev->dma_handle);
- out_5:
+free_cmap:
fb_dealloc_cmap(&fbdev->info.cmap);
- out_6:
+free_palette:
kfree(fbdev->info.pseudo_palette);
- out_7:
+free_fb_mem:
+ dma_free_coherent(NULL, fbdev->fb_len, fbdev->fb_mem,
+ fbdev->dma_handle);
+free_ppi_pins:
peripheral_free_list(ppi_pins);
- out_8:
+free_gpio:
+ if (ANOMALY_05000400)
+ gpio_free(P_IDENT(P_PPI0_FS3));
+free_fbdev:
kfree(fbdev);
return ret;
--
1.7.10
^ permalink raw reply related
* Re: [PATCH v2] video: bfin_adv7393fb: Fix cleanup code
From: Dan Carpenter @ 2012-05-29 6:21 UTC (permalink / raw)
To: Emil Goode; +Cc: FlorianSchandinat, linux-fbdev, linux-kernel, kernel-janitors
In-Reply-To: <1338224091-8322-1-git-send-email-emilgoode@gmail.com>
On Mon, May 28, 2012 at 06:54:51PM +0200, Emil Goode wrote:
> This patch fixes the cleanup code of the bfin_adv7393_fb_probe
> function.
>
> 1) The resources were not freed in the order that we allocated them
> so we call dma_free_coherent() before it was allocated.
> 2) The labels weren't in the right place which also meant that we
> freed resources that weren't allocated.
> 3) We should free gpio_free(P_IDENT(P_PPI0_FS3)) before returning.
> 4) Lets change the label names into something more meaningful.
>
> Signed-off-by: Emil Goode <emilgoode@gmail.com>
> ---
Yep. Looks great.
I see the goto free_ppi_pins; I look up a couple lines. I see that
yeah, it makes sense to free the ppi_pins. I hit the '*' key in vim
I see that the label does exactly what it says. Move on.
Bing. Bang. Boom.
regards,
dan carpenter
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox