* Re: [PATCH v10 5/6] devicetree: Add vendor prefix for Skyworks Solutions, Inc.
From: Lee Jones @ 2014-12-18 8:29 UTC (permalink / raw)
To: gyungoh
Cc: sameo, grant.likely, robh+dt, jg1.han, cooloney, pawel.moll,
mark.rutland, ijc+devicetree, galak, trivial, akpm, jic23,
thomas.petazzoni, ktsai, hs, stwiss.opensource, matti.vaittinen,
broonie, jason, heiko, shawn.guo, florian.vaussard, andrew,
antonynpavlov, hytszk, plagnioj, tomi.valkeinen, jack.yoo,
linux-fbdev, linux-kernel, devicetree
In-Reply-To: <1418881671-3495-6-git-send-email-jack.yoo@skyworksinc.com>
On Thu, 18 Dec 2014, gyungoh@gmail.com wrote:
> From: Gyungoh Yoo <jack.yoo@skyworksinc.com>
>
> Signed-off-by: Gyungoh Yoo <jack.yoo@skyworksinc.com>
> ---
> Changes v10:
> Nothing
>
> Changes v9:
> Nothing
>
> Changes v8:
> Nothing
>
> Changes v7:
> Nothing
>
> Changes v6:
> Nothing
>
> Changes v5:
> Nothing
>
> Changes v4:
> Nothing
>
> Changes v3:
> Nothing
Changes v3 => v10:
Nothing
... might be a little more succinct.
> Changes v2:
> Added vendor prefix for Skyworks Solutions, Inc.
>
> Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
> 1 file changed, 1 insertion(+)
Acked-by: Lee Jones <lee.jones@linaro.org>
> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
> index a344ec2..68143f0 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.txt
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
> @@ -140,6 +140,7 @@ sii Seiko Instruments, Inc.
> silergy Silergy Corp.
> sirf SiRF Technology, Inc.
> sitronix Sitronix Technology Corporation
> +skyworks Skyworks Solutions, Inc.
> smsc Standard Microsystems Corporation
> snps Synopsys, Inc.
> solidrun SolidRun
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [PATCH v10 4/6] devicetree: backlight: Add new SKY81452 backlight binding
From: Lee Jones @ 2014-12-18 8:30 UTC (permalink / raw)
To: gyungoh
Cc: sameo, grant.likely, robh+dt, jg1.han, cooloney, pawel.moll,
mark.rutland, ijc+devicetree, galak, trivial, akpm, jic23,
thomas.petazzoni, ktsai, hs, stwiss.opensource, matti.vaittinen,
broonie, jason, heiko, shawn.guo, florian.vaussard, andrew,
antonynpavlov, hytszk, plagnioj, tomi.valkeinen, jack.yoo,
linux-fbdev, linux-kernel, devicetree
In-Reply-To: <1418881671-3495-5-git-send-email-jack.yoo@skyworksinc.com>
DT Ack please.
> From: Gyungoh Yoo <jack.yoo@skyworksinc.com>
>
> Signed-off-by: Gyungoh Yoo <jack.yoo@skyworksinc.com>
> Acked-by: Bryan Wu <cooloney@gmail.com>
> ---
[...]
> Changes v8:
> Renamed property names for backlight with vendor prefix
> Modified gpio-enable property to generic property for GPIO
> Made up the example for backlight DT
[...]
> Changes v2:
> Added reg attribute for I2C slave address
>
> .../video/backlight/sky81452-backlight.txt | 29 ++++++++++++++++++++++
> 1 file changed, 29 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/video/backlight/sky81452-backlight.txt
>
> diff --git a/Documentation/devicetree/bindings/video/backlight/sky81452-backlight.txt b/Documentation/devicetree/bindings/video/backlight/sky81452-backlight.txt
> new file mode 100644
> index 0000000..8daebf5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/video/backlight/sky81452-backlight.txt
> @@ -0,0 +1,29 @@
> +SKY81452-backlight bindings
> +
> +Required properties:
> +- compatible : Must be "skyworks,sky81452-backlight"
> +
> +Optional properties:
> +- name : Name of backlight device. Default is 'lcd-backlight'.
> +- gpios : GPIO to use to EN pin.
> + See Documentation/devicetree/bindings/gpio/gpio.txt
> +- skyworks,en-channels : Enable mask for current sink channel 1 to 6.
> +- skyworks,ignore-pwm : Ignore both PWM input
> +- skyworks,dpwm-mode : Enable DPWM dimming mode, otherwise Analog dimming.
> +- skyworks,phase-shift : Enable phase shift mode
> +- skyworks,ovp-level : Over-voltage protection level.
> + Should be between 14 or 28V.
> +- skyworks,short-detection-threshold : It should be one of 4, 5, 6 and 7V.
> +- skyworks,current-limit : It should be 2300mA or 2750mA.
These need a DT Ack (if they are still alive).
> +Example:
> +
> + backlight {
> + compatible = "skyworks,sky81452-backlight";
> + name = "pwm-backlight";
> + skyworks,en-channels = <0x3f>;
> + skyworks,ignore-pwm;
> + skyworks,phase-shift;
> + skyworks,ovp-level = <20>;
> + skyworks,current-limit = <2300>;
> + };
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [PATCH v10 3/6] devicetree: mfd: Add new SKY81452 mfd binding
From: Lee Jones @ 2014-12-18 8:32 UTC (permalink / raw)
To: gyungoh
Cc: sameo, grant.likely, robh+dt, jg1.han, cooloney, pawel.moll,
mark.rutland, ijc+devicetree, galak, trivial, akpm, jic23,
thomas.petazzoni, ktsai, hs, stwiss.opensource, matti.vaittinen,
broonie, jason, heiko, shawn.guo, florian.vaussard, andrew,
antonynpavlov, hytszk, plagnioj, tomi.valkeinen, jack.yoo,
linux-fbdev, linux-kernel, devicetree
In-Reply-To: <1418881671-3495-4-git-send-email-jack.yoo@skyworksinc.com>
On Thu, 18 Dec 2014, gyungoh@gmail.com wrote:
> From: Gyungoh Yoo <jack.yoo@skyworksinc.com>
>
> Signed-off-by: Gyungoh Yoo <jack.yoo@skyworksinc.com>
> ---
> Changes v10:
> Nothing
>
> Changes v9:
> Nothing
>
> Changes v8:
> Made up the example for backlight DT
>
> Changes v7:
> Nothing
>
> Changes v6:
> Nothing
>
> Changes v5:
> Changed DT for regulator : 'lout' node should be defined under 'regulator'
> Removed compatible string from sky81452-regulator driver
>
> Changes v4:
> Nothing
>
> Changes v3:
> Nothing
>
> Changes v2:
> Added reg attribute for I2C slave address
>
> Documentation/devicetree/bindings/mfd/sky81452.txt | 36 ++++++++++++++++++++++
> 1 file changed, 36 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mfd/sky81452.txt
>
> diff --git a/Documentation/devicetree/bindings/mfd/sky81452.txt b/Documentation/devicetree/bindings/mfd/sky81452.txt
> new file mode 100644
> index 0000000..ab71473
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/sky81452.txt
> @@ -0,0 +1,36 @@
> +SKY81452 bindings
> +
> +Required properties:
> +- compatible : Must be "skyworks,sky81452"
> +- reg : I2C slave address
> +
> +Required child nodes:
> +- backlight : container node for backlight following the binding
> + in video/backlight/sky81452-backlight.txt
> +- regulator : container node for regulators following the binding
> + in regulator/sky81452-regulator.txt
For these MFD parts.
Acked-by: Lee Jones <lee.jones@linaro.org>
[Don't forget to apply the Acks you receive on subsequent submissions]
The Backlight parts need closer inspection by the DT people.
> +Example:
> +
> + sky81452@2c {
> + compatible = "skyworks,sky81452";
> + reg = <0x2c>;
> +
> + backlight {
> + compatible = "skyworks,sky81452-backlight";
> + name = "pwm-backlight";
> + skyworks,en-channels = <0x3f>;
> + skyworks,ignore-pwm;
> + skyworks,phase-shift;
> + skyworks,ovp-level = <20>;
> + skyworks,current-limit = <2300>;
> + };
> +
> + regulator {
> + lout {
> + regulator-name = "sky81452-lout";
> + regulator-min-microvolt = <4500000>;
> + regulator-max-microvolt = <8000000>;
> + };
> + };
> + };
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: Weird/Unneeded call to msleep in exynos_mipi_dsi_wr_data in exynos_mipi_dsi_common.c
From: Krzysztof Kozłowski @ 2014-12-18 10:26 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <54920A54.2060608@gmail.com>
On 17.12.2014 23:57, nick wrote:
> Greetings Fellow Maintainers,
> Sorry if I wasting your time but it seems there is a unneeded call to msleep
Hi,
1. Please describe exactly why do you think this is not needed.
2. Do you have Exynos-based hardware to test your changes?
Best regards,
Krzysztof
> and rather trivial fix me to fix in the file,exynos_mipi_dsi_common.c for the function,exynos_mipi_dsi_wr_data . If there is a valid reason for this call please let me known when any of you have some free time. Otherwise I will send in a patch to remove this unneeded
> call to msleep.
> Thanks,
> Nick
^ permalink raw reply
* [PATCH] video/logo: prevent use of logos after they have been freed
From: Tomi Valkeinen @ 2014-12-18 11:57 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
The fbdev logo seems to be rather messed up when deferred probing is happening.
This is a quick fix for the problem.
The code in the logo.c is quite scary, using __init_refok to silence warnings
about accessing __initdata from non-init context.
Another option would be to make the logos not __initdata, but I think that's
not acceptable.
Any ideas for a nicer way to fix this issue?
Tomi
Tomi Valkeinen (1):
video/logo: prevent use of logos after they have been freed
drivers/video/logo/logo.c | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)
--
2.2.0
^ permalink raw reply
* [PATCH] video/logo: prevent use of logos after they have been freed
From: Tomi Valkeinen @ 2014-12-18 11:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1418903842-22450-1-git-send-email-tomi.valkeinen@ti.com>
If the probe of an fb driver has been deferred due to missing
dependencies, and the probe is later ran when a module is loaded, the
fbdev framework will try to find a logo to use.
However, the logos are __initdata, and have already been freed. This
causes sometimes page faults, if the logo memory is not mapped,
sometimes other random crashes as the logo data is invalid, and
sometimes nothing, if the fbdev decides to reject the logo (e.g. the
random value depicting the logo's height is too big).
This patch adds a late_initcall function to mark the logos as freed. In
reality the logos are freed later, and fbdev probe may be ran between
this late_initcall and the freeing of the logos. In that case we will
miss drawing the logo, even if it would be possible.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
drivers/video/logo/logo.c | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/drivers/video/logo/logo.c b/drivers/video/logo/logo.c
index 940cd196eef5..10fbfd8ab963 100644
--- a/drivers/video/logo/logo.c
+++ b/drivers/video/logo/logo.c
@@ -21,6 +21,21 @@ static bool nologo;
module_param(nologo, bool, 0);
MODULE_PARM_DESC(nologo, "Disables startup logo");
+/*
+ * Logos are located in the initdata, and will be freed in kernel_init.
+ * Use late_init to mark the logos as freed to prevent any further use.
+ */
+
+static bool logos_freed;
+
+static int __init fb_logo_late_init(void)
+{
+ logos_freed = true;
+ return 0;
+}
+
+late_initcall(fb_logo_late_init);
+
/* logo's are marked __initdata. Use __init_refok to tell
* modpost that it is intended that this function uses data
* marked __initdata.
@@ -29,7 +44,7 @@ const struct linux_logo * __init_refok fb_find_logo(int depth)
{
const struct linux_logo *logo = NULL;
- if (nologo)
+ if (nologo || logos_freed)
return NULL;
if (depth >= 1) {
--
2.2.0
^ permalink raw reply related
* Re: Weird/Unneeded call to msleep in exynos_mipi_dsi_wr_data in exynos_mipi_dsi_common.c
From: Lucas Stach @ 2014-12-18 13:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5492D80E.3010906@gmail.com>
Am Donnerstag, den 18.12.2014, 08:35 -0500 schrieb nick:
> Krzysztof,
> If we look at the code for this function, it already is handling the data correctly. In addition the locks
> seem to be better protection then msleep. Further more is no reason for this delay as we are neither resetting
> the hardware or waiting for the hardware here so why is it needed? I don't have Exynos based hardware lying
> around through to test it.
If you can't test it, don't touch it. It's that simple.
Regards,
Lucas
--
Pengutronix e.K. | Lucas Stach |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* Re: [PATCH] video/logo: prevent use of logos after they have been freed
From: Geert Uytterhoeven @ 2014-12-18 13:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1418903842-22450-2-git-send-email-tomi.valkeinen@ti.com>
Hi Tomi,
On Thu, Dec 18, 2014 at 12:57 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> If the probe of an fb driver has been deferred due to missing
> dependencies, and the probe is later ran when a module is loaded, the
> fbdev framework will try to find a logo to use.
>
> However, the logos are __initdata, and have already been freed. This
> causes sometimes page faults, if the logo memory is not mapped,
> sometimes other random crashes as the logo data is invalid, and
> sometimes nothing, if the fbdev decides to reject the logo (e.g. the
> random value depicting the logo's height is too big).
>
> This patch adds a late_initcall function to mark the logos as freed. In
> reality the logos are freed later, and fbdev probe may be ran between
> this late_initcall and the freeing of the logos. In that case we will
> miss drawing the logo, even if it would be possible.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
> drivers/video/logo/logo.c | 17 ++++++++++++++++-
> 1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/video/logo/logo.c b/drivers/video/logo/logo.c
> index 940cd196eef5..10fbfd8ab963 100644
> --- a/drivers/video/logo/logo.c
> +++ b/drivers/video/logo/logo.c
> @@ -21,6 +21,21 @@ static bool nologo;
> module_param(nologo, bool, 0);
> MODULE_PARM_DESC(nologo, "Disables startup logo");
>
> +/*
> + * Logos are located in the initdata, and will be freed in kernel_init.
> + * Use late_init to mark the logos as freed to prevent any further use.
> + */
> +
> +static bool logos_freed;
> +
> +static int __init fb_logo_late_init(void)
> +{
> + logos_freed = true;
Just set nologo to true?
> + return 0;
> +}
> +
> +late_initcall(fb_logo_late_init);
Hmm...
> +
> /* logo's are marked __initdata. Use __init_refok to tell
> * modpost that it is intended that this function uses data
> * marked __initdata.
> @@ -29,7 +44,7 @@ const struct linux_logo * __init_refok fb_find_logo(int depth)
> {
> const struct linux_logo *logo = NULL;
>
> - if (nologo)
> + if (nologo || logos_freed)
A long time ago (ibefore 2.1.124), we used to have a test on initmem_freed
here. But that variable no longer exists.
Perhaps you can check system_state? That's variable is changed from
SYSTEM_BOOTING to SYSTEM_RUNNING in kernel_init(), after freeing
initmem.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* msync on fbdev
From: Martin Jackson @ 2014-12-18 13:55 UTC (permalink / raw)
To: linux-fbdev
Dear fbdev developers,
This relates to the core implementation of fbdev with the
CONFIG_FB_DEFERRED_IO option enabled.
I believe I have found a bug when doing msync(2) on a framebuffer
device. We are using an old (2.6.37) kernel on an embedded plaform,
but if I look at the current torvalds kernel (fbdev kernel looks
rather old!?), the bug seems to still be there.
When looking at the msync(2) man page, msync is meant to return 0 on
success and -1 on failure, however we are seeing it return 1, which is
undocumented and in my opinion wrong.
This is because the fb_deferred_io_fsync method returns the code from
'schedule_delayed_work', which is either 0 or 1 depending on whether
the work was already scheduled, leading to the possibility that msync
returns the value 1 to userland.
Best regards,
Martin Jackson
^ permalink raw reply
* Re: [PATCH] video/logo: prevent use of logos after they have been freed
From: Tomi Valkeinen @ 2014-12-18 13:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAMuHMdX9GB_ikDsGNLTgkDuAeRy81a-A9CVAFmX7Taab8_Z=oQ@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3312 bytes --]
On 18/12/14 15:46, Geert Uytterhoeven wrote:
> Hi Tomi,
>
> On Thu, Dec 18, 2014 at 12:57 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>> If the probe of an fb driver has been deferred due to missing
>> dependencies, and the probe is later ran when a module is loaded, the
>> fbdev framework will try to find a logo to use.
>>
>> However, the logos are __initdata, and have already been freed. This
>> causes sometimes page faults, if the logo memory is not mapped,
>> sometimes other random crashes as the logo data is invalid, and
>> sometimes nothing, if the fbdev decides to reject the logo (e.g. the
>> random value depicting the logo's height is too big).
>>
>> This patch adds a late_initcall function to mark the logos as freed. In
>> reality the logos are freed later, and fbdev probe may be ran between
>> this late_initcall and the freeing of the logos. In that case we will
>> miss drawing the logo, even if it would be possible.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
>> ---
>> drivers/video/logo/logo.c | 17 ++++++++++++++++-
>> 1 file changed, 16 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/video/logo/logo.c b/drivers/video/logo/logo.c
>> index 940cd196eef5..10fbfd8ab963 100644
>> --- a/drivers/video/logo/logo.c
>> +++ b/drivers/video/logo/logo.c
>> @@ -21,6 +21,21 @@ static bool nologo;
>> module_param(nologo, bool, 0);
>> MODULE_PARM_DESC(nologo, "Disables startup logo");
>>
>> +/*
>> + * Logos are located in the initdata, and will be freed in kernel_init.
>> + * Use late_init to mark the logos as freed to prevent any further use.
>> + */
>> +
>> +static bool logos_freed;
>> +
>> +static int __init fb_logo_late_init(void)
>> +{
>> + logos_freed = true;
>
> Just set nologo to true?
That was my first thought, but then I started to wonder if it's possible
that someone would set it to false afterwards. I'm not quite sure how,
as the module_param parsing should happen early. But I just wanted to
play safe.
>
>> + return 0;
>> +}
>> +
>> +late_initcall(fb_logo_late_init);
>
> Hmm...
>
>> +
>> /* logo's are marked __initdata. Use __init_refok to tell
>> * modpost that it is intended that this function uses data
>> * marked __initdata.
>> @@ -29,7 +44,7 @@ const struct linux_logo * __init_refok fb_find_logo(int depth)
>> {
>> const struct linux_logo *logo = NULL;
>>
>> - if (nologo)
>> + if (nologo || logos_freed)
>
> A long time ago (ibefore 2.1.124), we used to have a test on initmem_freed
> here. But that variable no longer exists.
I guess it was removed because it was considered a hack? =) The
functions accessing __initdata should be __init.
> Perhaps you can check system_state? That's variable is changed from
> SYSTEM_BOOTING to SYSTEM_RUNNING in kernel_init(), after freeing
> initmem.
Hmm maybe. But there's a time period between free_initmem() and setting
system_state to SYSTEM_RUNNING. Calling fb_find_logo() between those
would again break. I don't know if it's possible for fb_find_logo to be
called there in practice, but... Again, I'd rather play it safe. It's
not nice to debug crashes that happen inside the console code, as you
won't see the crashes on the console.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: Weird/Unneeded call to msleep in exynos_mipi_dsi_wr_data in exynos_mipi_dsi_common.c
From: Tomi Valkeinen @ 2014-12-18 14:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5492DB1A.2020704@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1022 bytes --]
On 18/12/14 15:48, nick wrote:
> Lucas,
> That's fair do you known of anyone who does have the hardware so we can test my patch. If you do then we can get this fixed rather
> easily.
> Cheers Nick
>
> On 2014-12-18 08:39 AM, Lucas Stach wrote:
>> Am Donnerstag, den 18.12.2014, 08:35 -0500 schrieb nick:
>>> Krzysztof,
>>> If we look at the code for this function, it already is handling the data correctly. In addition the locks
>>> seem to be better protection then msleep. Further more is no reason for this delay as we are neither resetting
>>> the hardware or waiting for the hardware here so why is it needed? I don't have Exynos based hardware lying
>>> around through to test it.
>>
>> If you can't test it, don't touch it. It's that simple.
There seems to be multiple msleep(20)s in exynos_mipi_dsi_common.c, a
few with /* FIXME */ and a few without any comments. Looks like bad (but
relatively harmless) code to me, but as Lucas said, if you can't test
it, don't touch it.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* [PATCH 1/6] tridentfb: fix hang on Blade3D with CONFIG_CC_OPTIMIZE_FOR_SIZE
From: Ondrej Zary @ 2014-12-18 21:39 UTC (permalink / raw)
To: Krzysztof Helt; +Cc: linux-fbdev, Kernel development list
When the kernel is compiled with -Os (CONFIG_CC_OPTIMIZE_FOR_SIZE), tridentfb
hangs the machine upon load with Blade3D cards unless acceleration is disabled.
This is caused by memcpy() which copies data byte-by-byte (rep movsb) when
compiled with -Os. The card does not like that - it requires 32-bit access.
Use iowrite_32() instead.
Signed-off-by: Ondrej Zary <linux@rainbow-software.org>
---
drivers/video/fbdev/tridentfb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/tridentfb.c b/drivers/video/fbdev/tridentfb.c
index 7ed9a22..7429713 100644
--- a/drivers/video/fbdev/tridentfb.c
+++ b/drivers/video/fbdev/tridentfb.c
@@ -226,7 +226,7 @@ static void blade_image_blit(struct tridentfb_par *par, const char *data,
writemmr(par, DST1, point(x, y));
writemmr(par, DST2, point(x + w - 1, y + h - 1));
- memcpy(par->io_virt + 0x10000, data, 4 * size);
+ iowrite32_rep(par->io_virt + 0x10000, data, size);
}
static void blade_copy_rect(struct tridentfb_par *par,
--
Ondrej Zary
^ permalink raw reply related
* [PATCH] tridentfb: fix hang on Blade3D with CONFIG_CC_OPTIMIZE_FOR_SIZE
From: Ondrej Zary @ 2014-12-18 21:41 UTC (permalink / raw)
To: Krzysztof Helt; +Cc: linux-fbdev, Kernel development list
When the kernel is compiled with -Os (CONFIG_CC_OPTIMIZE_FOR_SIZE), tridentfb
hangs the machine upon load with Blade3D cards unless acceleration is disabled.
This is caused by memcpy() which copies data byte-by-byte (rep movsb) when
compiled with -Os. The card does not like that - it requires 32-bit access.
Use iowrite_32() instead.
Signed-off-by: Ondrej Zary <linux@rainbow-software.org>
---
drivers/video/fbdev/tridentfb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/tridentfb.c b/drivers/video/fbdev/tridentfb.c
index 7ed9a22..7429713 100644
--- a/drivers/video/fbdev/tridentfb.c
+++ b/drivers/video/fbdev/tridentfb.c
@@ -226,7 +226,7 @@ static void blade_image_blit(struct tridentfb_par *par, const char *data,
writemmr(par, DST1, point(x, y));
writemmr(par, DST2, point(x + w - 1, y + h - 1));
- memcpy(par->io_virt + 0x10000, data, 4 * size);
+ iowrite32_rep(par->io_virt + 0x10000, data, size);
}
static void blade_copy_rect(struct tridentfb_par *par,
--
Ondrej Zary
^ permalink raw reply related
* [PATCH v2 0/7] imx-drm: ipuv3-crtc: Implement mode_fixup
From: Steve Longerbeam @ 2014-12-19 2:00 UTC (permalink / raw)
To: dri-devel, linux-kernel, linux-fbdev, David Airlie, Philipp Zabel,
Jean-Christophe Plagniol-Villard, Tomi Valkeinen
Cc: Russell King, Fabio Estevam, Shawn Guo, Denis Carikli, Jiada Wang,
Steve Longerbeam
This patchset implements ->mode_fixup() in the imx ipuv3-crtc driver,
using a new support function ipu_di_adjust_videomode(). This new
function needs to be subsystem independent, so it accepts a video
mode as a 'struct videomode'. Hence ipu-crtc ->mode_fixup() needs
another support function to convert a drm_display_mode to a videomode
before passing the mode to ipu_di_adjust_videomode() for fixup.
Also some related code cleanup: 'struct ipu_di_signal_cfg' should
use 'struct videomode' for mode timings.
Jiada Wang (1):
gpu: ipu-di: Add ipu_di_adjust_videomode()
Steve Longerbeam (6):
gpu: ipu-di: remove some non-functional code
drm_modes: add videomode_from_drm_display_mode
imx-drm: ipuv3-crtc: Implement mode_fixup
imx-drm: encoder prepare/mode_set must use adjusted mode
gpu: ipu-v3: Use videomode in struct ipu_di_signal_cfg
gpu: ipu-di: Switch to DIV_ROUND_CLOSEST for DI clock divider calc
drivers/gpu/drm/drm_modes.c | 40 +++++++++++
drivers/gpu/drm/imx/imx-hdmi.c | 4 +-
drivers/gpu/drm/imx/imx-ldb.c | 6 +-
drivers/gpu/drm/imx/imx-tve.c | 4 +-
drivers/gpu/drm/imx/ipuv3-crtc.c | 38 +++++-----
drivers/gpu/drm/imx/parallel-display.c | 4 +-
drivers/gpu/ipu-v3/ipu-di.c | 121 +++++++++++++++++++-------------
include/drm/drm_modes.h | 2 +
include/video/imx-ipu-v3.h | 21 ++----
9 files changed, 147 insertions(+), 93 deletions(-)
--
1.7.9.5
^ permalink raw reply
* [PATCH v2 1/7] gpu: ipu-di: Add ipu_di_adjust_videomode()
From: Steve Longerbeam @ 2014-12-19 2:00 UTC (permalink / raw)
To: dri-devel, linux-kernel, linux-fbdev, David Airlie, Philipp Zabel,
Jean-Christophe Plagniol-Villard, Tomi Valkeinen
Cc: Russell King, Fabio Estevam, Shawn Guo, Denis Carikli, Jiada Wang,
Deepak Das, Steve Longerbeam
In-Reply-To: <1418954426-21909-1-git-send-email-steve_longerbeam@mentor.com>
From: Jiada Wang <jiada_wang@mentor.com>
On some monitors, high resolution modes are not working, exhibiting
pixel column truncation problems (for example, 1280x1024 displays as
1280x1022).
The function ipu_di_adjust_videomode() aims to fix these issues by
adjusting a passed videomode to IPU restrictions. The function can
be called from the drm_crtc_helper_funcs->mode_fixup() methods.
Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
Signed-off-by: Deepak Das <deepak_das@mentor.com>
Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
---
drivers/gpu/ipu-v3/ipu-di.c | 29 +++++++++++++++++++++++++++++
include/video/imx-ipu-v3.h | 2 ++
2 files changed, 31 insertions(+)
diff --git a/drivers/gpu/ipu-v3/ipu-di.c b/drivers/gpu/ipu-v3/ipu-di.c
index c490ba4..46f9570 100644
--- a/drivers/gpu/ipu-v3/ipu-di.c
+++ b/drivers/gpu/ipu-v3/ipu-di.c
@@ -511,6 +511,35 @@ static void ipu_di_config_clock(struct ipu_di *di,
clk_get_rate(di->clk_di_pixel) / (clkgen0 >> 4));
}
+/*
+ * This function is called to adjust a video mode to IPU restrictions.
+ * It is meant to be called from drm crtc mode_fixup() methods.
+ */
+int ipu_di_adjust_videomode(struct ipu_di *di, struct videomode *mode)
+{
+ u32 diff;
+
+ if (mode->vfront_porch >= 2)
+ return 0;
+
+ diff = 2 - mode->vfront_porch;
+
+ if (mode->vback_porch >= diff) {
+ mode->vfront_porch = 2;
+ mode->vback_porch -= diff;
+ } else if (mode->vsync_len > diff) {
+ mode->vfront_porch = 2;
+ mode->vsync_len = mode->vsync_len - diff;
+ } else {
+ dev_warn(di->ipu->dev, "failed to adjust videomode\n");
+ return -EINVAL;
+ }
+
+ dev_warn(di->ipu->dev, "videomode adapted for IPU restrictions\n");
+ return 0;
+}
+EXPORT_SYMBOL_GPL(ipu_di_adjust_videomode);
+
int ipu_di_init_sync_panel(struct ipu_di *di, struct ipu_di_signal_cfg *sig)
{
u32 reg;
diff --git a/include/video/imx-ipu-v3.h b/include/video/imx-ipu-v3.h
index c74bf4a..d333d54 100644
--- a/include/video/imx-ipu-v3.h
+++ b/include/video/imx-ipu-v3.h
@@ -17,6 +17,7 @@
#include <linux/bitmap.h>
#include <linux/fb.h>
#include <media/v4l2-mediabus.h>
+#include <video/videomode.h>
struct ipu_soc;
@@ -236,6 +237,7 @@ void ipu_di_put(struct ipu_di *);
int ipu_di_disable(struct ipu_di *);
int ipu_di_enable(struct ipu_di *);
int ipu_di_get_num(struct ipu_di *);
+int ipu_di_adjust_videomode(struct ipu_di *di, struct videomode *mode);
int ipu_di_init_sync_panel(struct ipu_di *, struct ipu_di_signal_cfg *sig);
/*
--
1.7.9.5
^ permalink raw reply related
* [PATCH v2 2/7] gpu: ipu-di: remove some non-functional code
From: Steve Longerbeam @ 2014-12-19 2:00 UTC (permalink / raw)
To: dri-devel, linux-kernel, linux-fbdev, David Airlie, Philipp Zabel,
Jean-Christophe Plagniol-Villard, Tomi Valkeinen
Cc: Russell King, Fabio Estevam, Shawn Guo, Denis Carikli, Jiada Wang,
Steve Longerbeam
In-Reply-To: <1418954426-21909-1-git-send-email-steve_longerbeam@mentor.com>
h_total and v_total were calculated in ipu_di_init_sync_panel()
but never actually used. Remove.
Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
---
drivers/gpu/ipu-v3/ipu-di.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/drivers/gpu/ipu-v3/ipu-di.c b/drivers/gpu/ipu-v3/ipu-di.c
index 46f9570..41df8d7 100644
--- a/drivers/gpu/ipu-v3/ipu-di.c
+++ b/drivers/gpu/ipu-v3/ipu-di.c
@@ -545,7 +545,6 @@ int ipu_di_init_sync_panel(struct ipu_di *di, struct ipu_di_signal_cfg *sig)
u32 reg;
u32 di_gen, vsync_cnt;
u32 div;
- u32 h_total, v_total;
dev_dbg(di->ipu->dev, "disp %d: panel size = %d x %d\n",
di->id, sig->width, sig->height);
@@ -553,11 +552,6 @@ int ipu_di_init_sync_panel(struct ipu_di *di, struct ipu_di_signal_cfg *sig)
if ((sig->v_sync_width = 0) || (sig->h_sync_width = 0))
return -EINVAL;
- h_total = sig->width + sig->h_sync_width + sig->h_start_width +
- sig->h_end_width;
- v_total = sig->height + sig->v_sync_width + sig->v_start_width +
- sig->v_end_width;
-
dev_dbg(di->ipu->dev, "Clocks: IPU %luHz DI %luHz Needed %luHz\n",
clk_get_rate(di->clk_ipu),
clk_get_rate(di->clk_di),
--
1.7.9.5
^ permalink raw reply related
* [PATCH v2 3/7] drm_modes: add videomode_from_drm_display_mode
From: Steve Longerbeam @ 2014-12-19 2:00 UTC (permalink / raw)
To: dri-devel, linux-kernel, linux-fbdev, David Airlie, Philipp Zabel,
Jean-Christophe Plagniol-Villard, Tomi Valkeinen
Cc: Russell King, Fabio Estevam, Shawn Guo, Denis Carikli, Jiada Wang,
Steve Longerbeam
In-Reply-To: <1418954426-21909-1-git-send-email-steve_longerbeam@mentor.com>
Add conversion from drm_display_mode to videomode.
Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
---
drivers/gpu/drm/drm_modes.c | 40 ++++++++++++++++++++++++++++++++++++++++
include/drm/drm_modes.h | 2 ++
2 files changed, 42 insertions(+)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 6d8b941..583a391 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -615,6 +615,46 @@ void drm_display_mode_from_videomode(const struct videomode *vm,
}
EXPORT_SYMBOL_GPL(drm_display_mode_from_videomode);
+/**
+ * videomode_from_drm_display_mode - fill in @vm using @dmode,
+ * @dmode: drm_display_mode structure to use as source
+ * @vm: videomode structure to use as destination
+ *
+ * Fills out @vm using the display mode specified in @dmode.
+ */
+void videomode_from_drm_display_mode(const struct drm_display_mode *dmode,
+ struct videomode *vm)
+{
+ vm->hactive = dmode->hdisplay;
+ vm->hfront_porch = dmode->hsync_start - dmode->hdisplay;
+ vm->hsync_len = dmode->hsync_end - dmode->hsync_start;
+ vm->hback_porch = dmode->htotal - dmode->hsync_end;
+
+ vm->vactive = dmode->vdisplay;
+ vm->vfront_porch = dmode->vsync_start - dmode->vdisplay;
+ vm->vsync_len = dmode->vsync_end - dmode->vsync_start;
+ vm->vback_porch = dmode->vtotal - dmode->vsync_end;
+
+ vm->pixelclock = dmode->clock * 1000;
+
+ vm->flags = 0;
+ if (dmode->flags & DRM_MODE_FLAG_PHSYNC)
+ vm->flags |= DISPLAY_FLAGS_HSYNC_HIGH;
+ else if (dmode->flags & DRM_MODE_FLAG_NHSYNC)
+ vm->flags |= DISPLAY_FLAGS_HSYNC_LOW;
+ if (dmode->flags & DRM_MODE_FLAG_PVSYNC)
+ vm->flags |= DISPLAY_FLAGS_VSYNC_HIGH;
+ else if (dmode->flags & DRM_MODE_FLAG_NVSYNC)
+ vm->flags |= DISPLAY_FLAGS_VSYNC_LOW;
+ if (dmode->flags & DRM_MODE_FLAG_INTERLACE)
+ vm->flags |= DISPLAY_FLAGS_INTERLACED;
+ if (dmode->flags & DRM_MODE_FLAG_DBLSCAN)
+ vm->flags |= DISPLAY_FLAGS_DOUBLESCAN;
+ if (dmode->flags & DRM_MODE_FLAG_DBLCLK)
+ vm->flags |= DISPLAY_FLAGS_DOUBLECLK;
+}
+EXPORT_SYMBOL_GPL(videomode_from_drm_display_mode);
+
#ifdef CONFIG_OF
/**
* of_get_drm_display_mode - get a drm_display_mode from devicetree
diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
index 91d0582..60c0144 100644
--- a/include/drm/drm_modes.h
+++ b/include/drm/drm_modes.h
@@ -197,6 +197,8 @@ struct drm_display_mode *drm_gtf_mode_complex(struct drm_device *dev,
int GTF_K, int GTF_2J);
void drm_display_mode_from_videomode(const struct videomode *vm,
struct drm_display_mode *dmode);
+void videomode_from_drm_display_mode(const struct drm_display_mode *dmode,
+ struct videomode *vm);
int of_get_drm_display_mode(struct device_node *np,
struct drm_display_mode *dmode,
int index);
--
1.7.9.5
^ permalink raw reply related
* [PATCH v2 4/7] imx-drm: ipuv3-crtc: Implement mode_fixup
From: Steve Longerbeam @ 2014-12-19 2:00 UTC (permalink / raw)
To: dri-devel, linux-kernel, linux-fbdev, David Airlie, Philipp Zabel,
Jean-Christophe Plagniol-Villard, Tomi Valkeinen
Cc: Russell King, Fabio Estevam, Shawn Guo, Denis Carikli, Jiada Wang,
Steve Longerbeam
In-Reply-To: <1418954426-21909-1-git-send-email-steve_longerbeam@mentor.com>
Ask the IPU display interface, via ipu_di_adjust_videomode(), to
adjust a video mode to meet any DI restrictions. The function takes
a subsystem independent videomode, so the drm_display_mode must be
converted to videomode first, and then the adjusted mode converted
back to a drm_display_mode.
Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
---
drivers/gpu/drm/imx/ipuv3-crtc.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c b/drivers/gpu/drm/imx/ipuv3-crtc.c
index 11e84a2..fb16026 100644
--- a/drivers/gpu/drm/imx/ipuv3-crtc.c
+++ b/drivers/gpu/drm/imx/ipuv3-crtc.c
@@ -242,6 +242,18 @@ static bool ipu_crtc_mode_fixup(struct drm_crtc *crtc,
const struct drm_display_mode *mode,
struct drm_display_mode *adjusted_mode)
{
+ struct ipu_crtc *ipu_crtc = to_ipu_crtc(crtc);
+ struct videomode vm;
+ int ret;
+
+ videomode_from_drm_display_mode(adjusted_mode, &vm);
+
+ ret = ipu_di_adjust_videomode(ipu_crtc->di, &vm);
+ if (ret)
+ return false;
+
+ drm_display_mode_from_videomode(&vm, adjusted_mode);
+
return true;
}
--
1.7.9.5
^ permalink raw reply related
* [PATCH v2 5/7] imx-drm: encoder prepare/mode_set must use adjusted mode
From: Steve Longerbeam @ 2014-12-19 2:00 UTC (permalink / raw)
To: dri-devel, linux-kernel, linux-fbdev, David Airlie, Philipp Zabel,
Jean-Christophe Plagniol-Villard, Tomi Valkeinen
Cc: Russell King, Fabio Estevam, Shawn Guo, Denis Carikli, Jiada Wang,
Steve Longerbeam
In-Reply-To: <1418954426-21909-1-git-send-email-steve_longerbeam@mentor.com>
The encoder ->prepare() and ->mode_set() methods need to use the
hw adjusted mode, not the original mode.
Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
---
drivers/gpu/drm/imx/imx-hdmi.c | 4 ++--
drivers/gpu/drm/imx/imx-ldb.c | 6 +++---
drivers/gpu/drm/imx/imx-tve.c | 4 ++--
drivers/gpu/drm/imx/parallel-display.c | 4 ++--
4 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/imx/imx-hdmi.c b/drivers/gpu/drm/imx/imx-hdmi.c
index aaec6b2..32116cc 100644
--- a/drivers/gpu/drm/imx/imx-hdmi.c
+++ b/drivers/gpu/drm/imx/imx-hdmi.c
@@ -1417,8 +1417,8 @@ static struct drm_encoder *imx_hdmi_connector_best_encoder(struct drm_connector
}
static void imx_hdmi_encoder_mode_set(struct drm_encoder *encoder,
- struct drm_display_mode *mode,
- struct drm_display_mode *adjusted_mode)
+ struct drm_display_mode *orig_mode,
+ struct drm_display_mode *mode)
{
struct imx_hdmi *hdmi = container_of(encoder, struct imx_hdmi, encoder);
diff --git a/drivers/gpu/drm/imx/imx-ldb.c b/drivers/gpu/drm/imx/imx-ldb.c
index 4662e00..5b9c875 100644
--- a/drivers/gpu/drm/imx/imx-ldb.c
+++ b/drivers/gpu/drm/imx/imx-ldb.c
@@ -168,7 +168,7 @@ static void imx_ldb_encoder_prepare(struct drm_encoder *encoder)
{
struct imx_ldb_channel *imx_ldb_ch = enc_to_imx_ldb_ch(encoder);
struct imx_ldb *ldb = imx_ldb_ch->ldb;
- struct drm_display_mode *mode = &encoder->crtc->mode;
+ struct drm_display_mode *mode = &encoder->crtc->hwmode;
u32 pixel_fmt;
unsigned long serial_clk;
unsigned long di_clk = mode->clock * 1000;
@@ -246,8 +246,8 @@ static void imx_ldb_encoder_commit(struct drm_encoder *encoder)
}
static void imx_ldb_encoder_mode_set(struct drm_encoder *encoder,
- struct drm_display_mode *mode,
- struct drm_display_mode *adjusted_mode)
+ struct drm_display_mode *orig_mode,
+ struct drm_display_mode *mode)
{
struct imx_ldb_channel *imx_ldb_ch = enc_to_imx_ldb_ch(encoder);
struct imx_ldb *ldb = imx_ldb_ch->ldb;
diff --git a/drivers/gpu/drm/imx/imx-tve.c b/drivers/gpu/drm/imx/imx-tve.c
index 42c651b..9709bf9a 100644
--- a/drivers/gpu/drm/imx/imx-tve.c
+++ b/drivers/gpu/drm/imx/imx-tve.c
@@ -312,8 +312,8 @@ static void imx_tve_encoder_prepare(struct drm_encoder *encoder)
}
static void imx_tve_encoder_mode_set(struct drm_encoder *encoder,
- struct drm_display_mode *mode,
- struct drm_display_mode *adjusted_mode)
+ struct drm_display_mode *orig_mode,
+ struct drm_display_mode *mode)
{
struct imx_tve *tve = enc_to_tve(encoder);
unsigned long rounded_rate;
diff --git a/drivers/gpu/drm/imx/parallel-display.c b/drivers/gpu/drm/imx/parallel-display.c
index 015a454..d0842a4 100644
--- a/drivers/gpu/drm/imx/parallel-display.c
+++ b/drivers/gpu/drm/imx/parallel-display.c
@@ -131,8 +131,8 @@ static void imx_pd_encoder_commit(struct drm_encoder *encoder)
}
static void imx_pd_encoder_mode_set(struct drm_encoder *encoder,
- struct drm_display_mode *mode,
- struct drm_display_mode *adjusted_mode)
+ struct drm_display_mode *orig_mode,
+ struct drm_display_mode *mode)
{
}
--
1.7.9.5
^ permalink raw reply related
* [PATCH v2 6/7] gpu: ipu-v3: Use videomode in struct ipu_di_signal_cfg
From: Steve Longerbeam @ 2014-12-19 2:00 UTC (permalink / raw)
To: dri-devel, linux-kernel, linux-fbdev, David Airlie, Philipp Zabel,
Jean-Christophe Plagniol-Villard, Tomi Valkeinen
Cc: Russell King, Fabio Estevam, Shawn Guo, Denis Carikli, Jiada Wang,
Steve Longerbeam
In-Reply-To: <1418954426-21909-1-git-send-email-steve_longerbeam@mentor.com>
This patch changes struct ipu_di_signal_cfg to use struct videomode
to define video timings and flags.
Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
---
drivers/gpu/drm/imx/ipuv3-crtc.c | 26 +++--------
drivers/gpu/ipu-v3/ipu-di.c | 89 ++++++++++++++++++++------------------
include/video/imx-ipu-v3.h | 19 ++------
3 files changed, 56 insertions(+), 78 deletions(-)
diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c b/drivers/gpu/drm/imx/ipuv3-crtc.c
index fb16026..1ca6492 100644
--- a/drivers/gpu/drm/imx/ipuv3-crtc.c
+++ b/drivers/gpu/drm/imx/ipuv3-crtc.c
@@ -158,35 +158,19 @@ static int ipu_crtc_mode_set(struct drm_crtc *crtc,
out_pixel_fmt = ipu_crtc->interface_pix_fmt;
- if (mode->flags & DRM_MODE_FLAG_INTERLACE)
- sig_cfg.interlaced = 1;
- if (mode->flags & DRM_MODE_FLAG_PHSYNC)
- sig_cfg.Hsync_pol = 1;
- if (mode->flags & DRM_MODE_FLAG_PVSYNC)
- sig_cfg.Vsync_pol = 1;
-
sig_cfg.enable_pol = 1;
sig_cfg.clk_pol = 0;
- sig_cfg.width = mode->hdisplay;
- sig_cfg.height = mode->vdisplay;
sig_cfg.pixel_fmt = out_pixel_fmt;
- sig_cfg.h_start_width = mode->htotal - mode->hsync_end;
- sig_cfg.h_sync_width = mode->hsync_end - mode->hsync_start;
- sig_cfg.h_end_width = mode->hsync_start - mode->hdisplay;
-
- sig_cfg.v_start_width = mode->vtotal - mode->vsync_end;
- sig_cfg.v_sync_width = mode->vsync_end - mode->vsync_start;
- sig_cfg.v_end_width = mode->vsync_start - mode->vdisplay;
- sig_cfg.pixelclock = mode->clock * 1000;
sig_cfg.clkflags = ipu_crtc->di_clkflags;
-
sig_cfg.v_to_h_sync = 0;
-
sig_cfg.hsync_pin = ipu_crtc->di_hsync_pin;
sig_cfg.vsync_pin = ipu_crtc->di_vsync_pin;
- ret = ipu_dc_init_sync(ipu_crtc->dc, ipu_crtc->di, sig_cfg.interlaced,
- out_pixel_fmt, mode->hdisplay);
+ videomode_from_drm_display_mode(mode, &sig_cfg.mode);
+
+ ret = ipu_dc_init_sync(ipu_crtc->dc, ipu_crtc->di,
+ mode->flags & DRM_MODE_FLAG_INTERLACE,
+ out_pixel_fmt, mode->hdisplay);
if (ret) {
dev_err(ipu_crtc->dev,
"initializing display controller failed with %d\n",
diff --git a/drivers/gpu/ipu-v3/ipu-di.c b/drivers/gpu/ipu-v3/ipu-di.c
index 41df8d7..d95fbd0 100644
--- a/drivers/gpu/ipu-v3/ipu-di.c
+++ b/drivers/gpu/ipu-v3/ipu-di.c
@@ -207,10 +207,10 @@ static void ipu_di_sync_config(struct ipu_di *di, struct di_sync_config *config,
static void ipu_di_sync_config_interlaced(struct ipu_di *di,
struct ipu_di_signal_cfg *sig)
{
- u32 h_total = sig->width + sig->h_sync_width +
- sig->h_start_width + sig->h_end_width;
- u32 v_total = sig->height + sig->v_sync_width +
- sig->v_start_width + sig->v_end_width;
+ u32 h_total = sig->mode.hactive + sig->mode.hsync_len +
+ sig->mode.hback_porch + sig->mode.hfront_porch;
+ u32 v_total = sig->mode.vactive + sig->mode.vsync_len +
+ sig->mode.vback_porch + sig->mode.vfront_porch;
u32 reg;
struct di_sync_config cfg[] = {
{
@@ -229,13 +229,13 @@ static void ipu_di_sync_config_interlaced(struct ipu_di *di,
}, {
.run_count = v_total / 2 - 1,
.run_src = DI_SYNC_HSYNC,
- .offset_count = sig->v_start_width,
+ .offset_count = sig->mode.vback_porch,
.offset_src = DI_SYNC_HSYNC,
.repeat_count = 2,
.cnt_clr_src = DI_SYNC_VSYNC,
}, {
.run_src = DI_SYNC_HSYNC,
- .repeat_count = sig->height / 2,
+ .repeat_count = sig->mode.vactive / 2,
.cnt_clr_src = 4,
}, {
.run_count = v_total - 1,
@@ -249,9 +249,9 @@ static void ipu_di_sync_config_interlaced(struct ipu_di *di,
.cnt_clr_src = DI_SYNC_VSYNC,
}, {
.run_src = DI_SYNC_CLK,
- .offset_count = sig->h_start_width,
+ .offset_count = sig->mode.hback_porch,
.offset_src = DI_SYNC_CLK,
- .repeat_count = sig->width,
+ .repeat_count = sig->mode.hactive,
.cnt_clr_src = 5,
}, {
.run_count = v_total - 1,
@@ -277,10 +277,10 @@ static void ipu_di_sync_config_interlaced(struct ipu_di *di,
static void ipu_di_sync_config_noninterlaced(struct ipu_di *di,
struct ipu_di_signal_cfg *sig, int div)
{
- u32 h_total = sig->width + sig->h_sync_width + sig->h_start_width +
- sig->h_end_width;
- u32 v_total = sig->height + sig->v_sync_width + sig->v_start_width +
- sig->v_end_width;
+ u32 h_total = sig->mode.hactive + sig->mode.hsync_len +
+ sig->mode.hback_porch + sig->mode.hfront_porch;
+ u32 v_total = sig->mode.vactive + sig->mode.vsync_len +
+ sig->mode.vback_porch + sig->mode.vfront_porch;
struct di_sync_config cfg[] = {
{
/* 1: INT_HSYNC */
@@ -294,27 +294,29 @@ static void ipu_di_sync_config_noninterlaced(struct ipu_di *di,
.offset_src = DI_SYNC_CLK,
.cnt_polarity_gen_en = 1,
.cnt_polarity_trigger_src = DI_SYNC_CLK,
- .cnt_down = sig->h_sync_width * 2,
+ .cnt_down = sig->mode.hsync_len * 2,
} , {
/* PIN3: VSYNC */
.run_count = v_total - 1,
.run_src = DI_SYNC_INT_HSYNC,
.cnt_polarity_gen_en = 1,
.cnt_polarity_trigger_src = DI_SYNC_INT_HSYNC,
- .cnt_down = sig->v_sync_width * 2,
+ .cnt_down = sig->mode.vsync_len * 2,
} , {
/* 4: Line Active */
.run_src = DI_SYNC_HSYNC,
- .offset_count = sig->v_sync_width + sig->v_start_width,
+ .offset_count = sig->mode.vsync_len +
+ sig->mode.vback_porch,
.offset_src = DI_SYNC_HSYNC,
- .repeat_count = sig->height,
+ .repeat_count = sig->mode.vactive,
.cnt_clr_src = DI_SYNC_VSYNC,
} , {
/* 5: Pixel Active, referenced by DC */
.run_src = DI_SYNC_CLK,
- .offset_count = sig->h_sync_width + sig->h_start_width,
+ .offset_count = sig->mode.hsync_len +
+ sig->mode.hback_porch,
.offset_src = DI_SYNC_CLK,
- .repeat_count = sig->width,
+ .repeat_count = sig->mode.hactive,
.cnt_clr_src = 5, /* Line Active */
} , {
/* unused */
@@ -339,9 +341,10 @@ static void ipu_di_sync_config_noninterlaced(struct ipu_di *di,
} , {
/* 3: Line Active */
.run_src = DI_SYNC_INT_HSYNC,
- .offset_count = sig->v_sync_width + sig->v_start_width,
+ .offset_count = sig->mode.vsync_len +
+ sig->mode.vback_porch,
.offset_src = DI_SYNC_INT_HSYNC,
- .repeat_count = sig->height,
+ .repeat_count = sig->mode.vactive,
.cnt_clr_src = 3 /* VSYNC */,
} , {
/* PIN4: HSYNC for VGA via TVEv2 on TQ MBa53 */
@@ -351,13 +354,14 @@ static void ipu_di_sync_config_noninterlaced(struct ipu_di *di,
.offset_src = DI_SYNC_CLK,
.cnt_polarity_gen_en = 1,
.cnt_polarity_trigger_src = DI_SYNC_CLK,
- .cnt_down = sig->h_sync_width * 2,
+ .cnt_down = sig->mode.hsync_len * 2,
} , {
/* 5: Pixel Active signal to DC */
.run_src = DI_SYNC_CLK,
- .offset_count = sig->h_sync_width + sig->h_start_width,
+ .offset_count = sig->mode.hsync_len +
+ sig->mode.hback_porch,
.offset_src = DI_SYNC_CLK,
- .repeat_count = sig->width,
+ .repeat_count = sig->mode.hactive,
.cnt_clr_src = 4, /* Line Active */
} , {
/* PIN6: VSYNC for VGA via TVEv2 on TQ MBa53 */
@@ -367,7 +371,7 @@ static void ipu_di_sync_config_noninterlaced(struct ipu_di *di,
.offset_src = DI_SYNC_INT_HSYNC,
.cnt_polarity_gen_en = 1,
.cnt_polarity_trigger_src = DI_SYNC_INT_HSYNC,
- .cnt_down = sig->v_sync_width * 2,
+ .cnt_down = sig->mode.vsync_len * 2,
} , {
/* PIN4: HSYNC for VGA via TVEv2 on i.MX53-QSB */
.run_count = h_total - 1,
@@ -376,7 +380,7 @@ static void ipu_di_sync_config_noninterlaced(struct ipu_di *di,
.offset_src = DI_SYNC_CLK,
.cnt_polarity_gen_en = 1,
.cnt_polarity_trigger_src = DI_SYNC_CLK,
- .cnt_down = sig->h_sync_width * 2,
+ .cnt_down = sig->mode.hsync_len * 2,
} , {
/* PIN6: VSYNC for VGA via TVEv2 on i.MX53-QSB */
.run_count = v_total - 1,
@@ -385,7 +389,7 @@ static void ipu_di_sync_config_noninterlaced(struct ipu_di *di,
.offset_src = DI_SYNC_INT_HSYNC,
.cnt_polarity_gen_en = 1,
.cnt_polarity_trigger_src = DI_SYNC_INT_HSYNC,
- .cnt_down = sig->v_sync_width * 2,
+ .cnt_down = sig->mode.vsync_len * 2,
} , {
/* unused */
},
@@ -433,10 +437,11 @@ static void ipu_di_config_clock(struct ipu_di *di,
unsigned long in_rate;
unsigned div;
- clk_set_rate(clk, sig->pixelclock);
+ clk_set_rate(clk, sig->mode.pixelclock);
in_rate = clk_get_rate(clk);
- div = (in_rate + sig->pixelclock / 2) / sig->pixelclock;
+ div = (in_rate + sig->mode.pixelclock / 2) /
+ sig->mode.pixelclock;
if (div = 0)
div = 1;
@@ -454,10 +459,11 @@ static void ipu_di_config_clock(struct ipu_di *di,
unsigned div, error;
clkrate = clk_get_rate(di->clk_ipu);
- div = (clkrate + sig->pixelclock / 2) / sig->pixelclock;
+ div = (clkrate + sig->mode.pixelclock / 2) /
+ sig->mode.pixelclock;
rate = clkrate / div;
- error = rate / (sig->pixelclock / 1000);
+ error = rate / (sig->mode.pixelclock / 1000);
dev_dbg(di->ipu->dev, " IPU clock can give %lu with divider %u, error %d.%u%%\n",
rate, div, (signed)(error - 1000) / 10, error % 10);
@@ -473,10 +479,11 @@ static void ipu_di_config_clock(struct ipu_di *di,
clk = di->clk_di;
- clk_set_rate(clk, sig->pixelclock);
+ clk_set_rate(clk, sig->mode.pixelclock);
in_rate = clk_get_rate(clk);
- div = (in_rate + sig->pixelclock / 2) / sig->pixelclock;
+ div = (in_rate + sig->mode.pixelclock / 2) /
+ sig->mode.pixelclock;
if (div = 0)
div = 1;
@@ -504,7 +511,7 @@ static void ipu_di_config_clock(struct ipu_di *di,
ipu_di_write(di, val, DI_GENERAL);
dev_dbg(di->ipu->dev, "Want %luHz IPU %luHz DI %luHz using %s, %luHz\n",
- sig->pixelclock,
+ sig->mode.pixelclock,
clk_get_rate(di->clk_ipu),
clk_get_rate(di->clk_di),
clk = di->clk_di ? "DI" : "IPU",
@@ -547,15 +554,15 @@ int ipu_di_init_sync_panel(struct ipu_di *di, struct ipu_di_signal_cfg *sig)
u32 div;
dev_dbg(di->ipu->dev, "disp %d: panel size = %d x %d\n",
- di->id, sig->width, sig->height);
+ di->id, sig->mode.hactive, sig->mode.vactive);
- if ((sig->v_sync_width = 0) || (sig->h_sync_width = 0))
+ if ((sig->mode.vsync_len = 0) || (sig->mode.hsync_len = 0))
return -EINVAL;
dev_dbg(di->ipu->dev, "Clocks: IPU %luHz DI %luHz Needed %luHz\n",
clk_get_rate(di->clk_ipu),
clk_get_rate(di->clk_di),
- sig->pixelclock);
+ sig->mode.pixelclock);
mutex_lock(&di_mutex);
@@ -574,7 +581,7 @@ int ipu_di_init_sync_panel(struct ipu_di *di, struct ipu_di_signal_cfg *sig)
di_gen = ipu_di_read(di, DI_GENERAL) & DI_GEN_DI_CLK_EXT;
di_gen |= DI_GEN_DI_VSYNC_EXT;
- if (sig->interlaced) {
+ if (sig->mode.flags & DISPLAY_FLAGS_INTERLACED) {
ipu_di_sync_config_interlaced(di, sig);
/* set y_sel = 1 */
@@ -584,9 +591,9 @@ int ipu_di_init_sync_panel(struct ipu_di *di, struct ipu_di_signal_cfg *sig)
vsync_cnt = 7;
- if (sig->Hsync_pol)
+ if (sig->mode.flags & DISPLAY_FLAGS_HSYNC_HIGH)
di_gen |= DI_GEN_POLARITY_3;
- if (sig->Vsync_pol)
+ if (sig->mode.flags & DISPLAY_FLAGS_VSYNC_HIGH)
di_gen |= DI_GEN_POLARITY_2;
} else {
ipu_di_sync_config_noninterlaced(di, sig, div);
@@ -600,7 +607,7 @@ int ipu_di_init_sync_panel(struct ipu_di *di, struct ipu_di_signal_cfg *sig)
if (!(sig->hsync_pin = 2 && sig->vsync_pin = 3))
vsync_cnt = 6;
- if (sig->Hsync_pol) {
+ if (sig->mode.flags & DISPLAY_FLAGS_HSYNC_HIGH) {
if (sig->hsync_pin = 2)
di_gen |= DI_GEN_POLARITY_2;
else if (sig->hsync_pin = 4)
@@ -608,7 +615,7 @@ int ipu_di_init_sync_panel(struct ipu_di *di, struct ipu_di_signal_cfg *sig)
else if (sig->hsync_pin = 7)
di_gen |= DI_GEN_POLARITY_7;
}
- if (sig->Vsync_pol) {
+ if (sig->mode.flags & DISPLAY_FLAGS_VSYNC_HIGH) {
if (sig->vsync_pin = 3)
di_gen |= DI_GEN_POLARITY_3;
else if (sig->vsync_pin = 6)
diff --git a/include/video/imx-ipu-v3.h b/include/video/imx-ipu-v3.h
index d333d54..73390c1 100644
--- a/include/video/imx-ipu-v3.h
+++ b/include/video/imx-ipu-v3.h
@@ -33,28 +33,15 @@ enum ipuv3_type {
* Bitfield of Display Interface signal polarities.
*/
struct ipu_di_signal_cfg {
- unsigned datamask_en:1;
- unsigned interlaced:1;
- unsigned odd_field_first:1;
- unsigned clksel_en:1;
- unsigned clkidle_en:1;
unsigned data_pol:1; /* true = inverted */
unsigned clk_pol:1; /* true = rising edge */
unsigned enable_pol:1;
- unsigned Hsync_pol:1; /* true = active high */
- unsigned Vsync_pol:1;
- u16 width;
- u16 height;
+ struct videomode mode;
+
u32 pixel_fmt;
- u16 h_start_width;
- u16 h_sync_width;
- u16 h_end_width;
- u16 v_start_width;
- u16 v_sync_width;
- u16 v_end_width;
u32 v_to_h_sync;
- unsigned long pixelclock;
+
#define IPU_DI_CLKMODE_SYNC (1 << 0)
#define IPU_DI_CLKMODE_EXT (1 << 1)
unsigned long clkflags;
--
1.7.9.5
^ permalink raw reply related
* [PATCH v2 7/7] gpu: ipu-di: Switch to DIV_ROUND_CLOSEST for DI clock divider calc
From: Steve Longerbeam @ 2014-12-19 2:00 UTC (permalink / raw)
To: dri-devel, linux-kernel, linux-fbdev, David Airlie, Philipp Zabel,
Jean-Christophe Plagniol-Villard, Tomi Valkeinen
Cc: Russell King, Fabio Estevam, Shawn Guo, Denis Carikli, Jiada Wang,
Steve Longerbeam
In-Reply-To: <1418954426-21909-1-git-send-email-steve_longerbeam@mentor.com>
We can use the DIV_ROUND_CLOSEST() macro when calculating the DI
clock divider, rounded to nearest int.
Suggested-by: Philipp Zabel <p.zabel@pengutronix.de>
Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
---
drivers/gpu/ipu-v3/ipu-di.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/ipu-v3/ipu-di.c b/drivers/gpu/ipu-v3/ipu-di.c
index d95fbd0..b61d6be 100644
--- a/drivers/gpu/ipu-v3/ipu-di.c
+++ b/drivers/gpu/ipu-v3/ipu-di.c
@@ -440,8 +440,7 @@ static void ipu_di_config_clock(struct ipu_di *di,
clk_set_rate(clk, sig->mode.pixelclock);
in_rate = clk_get_rate(clk);
- div = (in_rate + sig->mode.pixelclock / 2) /
- sig->mode.pixelclock;
+ div = DIV_ROUND_CLOSEST(in_rate, sig->mode.pixelclock);
if (div = 0)
div = 1;
@@ -459,8 +458,7 @@ static void ipu_di_config_clock(struct ipu_di *di,
unsigned div, error;
clkrate = clk_get_rate(di->clk_ipu);
- div = (clkrate + sig->mode.pixelclock / 2) /
- sig->mode.pixelclock;
+ div = DIV_ROUND_CLOSEST(clkrate, sig->mode.pixelclock);
rate = clkrate / div;
error = rate / (sig->mode.pixelclock / 1000);
@@ -482,8 +480,7 @@ static void ipu_di_config_clock(struct ipu_di *di,
clk_set_rate(clk, sig->mode.pixelclock);
in_rate = clk_get_rate(clk);
- div = (in_rate + sig->mode.pixelclock / 2) /
- sig->mode.pixelclock;
+ div = DIV_ROUND_CLOSEST(in_rate, sig->mode.pixelclock);
if (div = 0)
div = 1;
--
1.7.9.5
^ permalink raw reply related
* Need Help to create frame buffer driver under Linux System
From: Ritesh Prajapati @ 2014-12-19 7:42 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <54927CAE.7060604@slscorp.com>
Hi,
I have one FPGA board which has DDR3 memory, HDMI Video Interface
as well as PCIe Interface. So, I want to use that board as graphics card
in my Linux System.
That FPGA board is connected via PCIe Interface in my system. I
have created one sample PCI Driver to enable pci device and get some
properties like BAR information for DDR3 memory.
I have also created frame buffer driver using virtual frame buffer
driver reference and mapped video memory with DDR3 memory which are
connected on FPGA board.
Can anyone helped me about how to manage multiple frame buffer in
DDR3 memory from frame buffer driver? Also please provide some more
information about how to create full functional frame buffer driver in
Linux.
Please let me know if need more information.
--
Regards,
Ritesh Prajapati,
System Level Solutions (India) Pvt.Ltd.
^ permalink raw reply
* Re: [PATCH v2 3/7] drm_modes: add videomode_from_drm_display_mode
From: Philipp Zabel @ 2014-12-19 11:03 UTC (permalink / raw)
To: Steve Longerbeam
Cc: Fabio Estevam, linux-fbdev, Steve Longerbeam, linux-kernel,
dri-devel, Denis Carikli, Tomi Valkeinen, Russell King,
Jean-Christophe Plagniol-Villard
In-Reply-To: <1418954426-21909-4-git-send-email-steve_longerbeam@mentor.com>
Am Donnerstag, den 18.12.2014, 18:00 -0800 schrieb Steve Longerbeam:
> Add conversion from drm_display_mode to videomode.
>
> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
> ---
> drivers/gpu/drm/drm_modes.c | 40 ++++++++++++++++++++++++++++++++++++++++
> include/drm/drm_modes.h | 2 ++
> 2 files changed, 42 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> index 6d8b941..583a391 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -615,6 +615,46 @@ void drm_display_mode_from_videomode(const struct videomode *vm,
> }
> EXPORT_SYMBOL_GPL(drm_display_mode_from_videomode);
>
> +/**
> + * videomode_from_drm_display_mode - fill in @vm using @dmode,
> + * @dmode: drm_display_mode structure to use as source
> + * @vm: videomode structure to use as destination
> + *
> + * Fills out @vm using the display mode specified in @dmode.
> + */
> +void videomode_from_drm_display_mode(const struct drm_display_mode *dmode,
> + struct videomode *vm)
> +{
> + vm->hactive = dmode->hdisplay;
> + vm->hfront_porch = dmode->hsync_start - dmode->hdisplay;
> + vm->hsync_len = dmode->hsync_end - dmode->hsync_start;
> + vm->hback_porch = dmode->htotal - dmode->hsync_end;
> +
> + vm->vactive = dmode->vdisplay;
> + vm->vfront_porch = dmode->vsync_start - dmode->vdisplay;
> + vm->vsync_len = dmode->vsync_end - dmode->vsync_start;
> + vm->vback_porch = dmode->vtotal - dmode->vsync_end;
> +
> + vm->pixelclock = dmode->clock * 1000;
> +
> + vm->flags = 0;
> + if (dmode->flags & DRM_MODE_FLAG_PHSYNC)
> + vm->flags |= DISPLAY_FLAGS_HSYNC_HIGH;
> + else if (dmode->flags & DRM_MODE_FLAG_NHSYNC)
> + vm->flags |= DISPLAY_FLAGS_HSYNC_LOW;
> + if (dmode->flags & DRM_MODE_FLAG_PVSYNC)
> + vm->flags |= DISPLAY_FLAGS_VSYNC_HIGH;
> + else if (dmode->flags & DRM_MODE_FLAG_NVSYNC)
> + vm->flags |= DISPLAY_FLAGS_VSYNC_LOW;
> + if (dmode->flags & DRM_MODE_FLAG_INTERLACE)
> + vm->flags |= DISPLAY_FLAGS_INTERLACED;
> + if (dmode->flags & DRM_MODE_FLAG_DBLSCAN)
> + vm->flags |= DISPLAY_FLAGS_DOUBLESCAN;
> + if (dmode->flags & DRM_MODE_FLAG_DBLCLK)
> + vm->flags |= DISPLAY_FLAGS_DOUBLECLK;
> +}
> +EXPORT_SYMBOL_GPL(videomode_from_drm_display_mode);
Is it ok for drm_modes to export a function that doesn't start with
drm_ ? We could just rename this to drm_display_mode_to_videomode if
necessary. I can fix it up as I apply it, but I'd like to know which is
preferred.
regards
Philipp
^ permalink raw reply
* Re: [PATCH v2 1/7] gpu: ipu-di: Add ipu_di_adjust_videomode()
From: Philipp Zabel @ 2014-12-19 11:07 UTC (permalink / raw)
To: Steve Longerbeam
Cc: Fabio Estevam, linux-fbdev, Steve Longerbeam, linux-kernel,
dri-devel, Denis Carikli, Tomi Valkeinen, Russell King,
Deepak Das, Jean-Christophe Plagniol-Villard
In-Reply-To: <1418954426-21909-2-git-send-email-steve_longerbeam@mentor.com>
Hi Steve,
Am Donnerstag, den 18.12.2014, 18:00 -0800 schrieb Steve Longerbeam:
> From: Jiada Wang <jiada_wang@mentor.com>
>
> On some monitors, high resolution modes are not working, exhibiting
> pixel column truncation problems (for example, 1280x1024 displays as
> 1280x1022).
>
> The function ipu_di_adjust_videomode() aims to fix these issues by
> adjusting a passed videomode to IPU restrictions. The function can
> be called from the drm_crtc_helper_funcs->mode_fixup() methods.
>
> Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
> Signed-off-by: Deepak Das <deepak_das@mentor.com>
> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
> ---
> drivers/gpu/ipu-v3/ipu-di.c | 29 +++++++++++++++++++++++++++++
> include/video/imx-ipu-v3.h | 2 ++
> 2 files changed, 31 insertions(+)
>
> diff --git a/drivers/gpu/ipu-v3/ipu-di.c b/drivers/gpu/ipu-v3/ipu-di.c
> index c490ba4..46f9570 100644
> --- a/drivers/gpu/ipu-v3/ipu-di.c
> +++ b/drivers/gpu/ipu-v3/ipu-di.c
> @@ -511,6 +511,35 @@ static void ipu_di_config_clock(struct ipu_di *di,
> clk_get_rate(di->clk_di_pixel) / (clkgen0 >> 4));
> }
>
> +/*
> + * This function is called to adjust a video mode to IPU restrictions.
> + * It is meant to be called from drm crtc mode_fixup() methods.
> + */
> +int ipu_di_adjust_videomode(struct ipu_di *di, struct videomode *mode)
> +{
> + u32 diff;
> +
> + if (mode->vfront_porch >= 2)
> + return 0;
> +
> + diff = 2 - mode->vfront_porch;
> +
> + if (mode->vback_porch >= diff) {
> + mode->vfront_porch = 2;
> + mode->vback_porch -= diff;
Should we also add
else if (mode->vback_porch >= 1 && mode->vsync_len > 1) {
mode->vback_porch--;
mode->vsync_len--;
here and maybe move the two "mode->vback_porch = 2" to a single place
below the conditional statement?
> + } else if (mode->vsync_len > diff) {
> + mode->vfront_porch = 2;
> + mode->vsync_len = mode->vsync_len - diff;
Why use "vback_porch -= diff" above, but not "vsync_len -= diff" here?
> + } else {
> + dev_warn(di->ipu->dev, "failed to adjust videomode\n");
> + return -EINVAL;
> + }
> +
> + dev_warn(di->ipu->dev, "videomode adapted for IPU restrictions\n");
Since we can return the adjusted mode to userspace now, I think this
should be dev_dbg.
regards
Philipp
^ permalink raw reply
* Re: msync on fbdev
From: Tomi Valkeinen @ 2014-12-19 11:59 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <CABMKUD9mtus77hdfPA5mAtwc8MCpyJQ-kvvvyVmKQuw2Tuo9mA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2389 bytes --]
On 18/12/14 15:55, Martin Jackson wrote:
> Dear fbdev developers,
>
> This relates to the core implementation of fbdev with the
> CONFIG_FB_DEFERRED_IO option enabled.
>
> I believe I have found a bug when doing msync(2) on a framebuffer
> device. We are using an old (2.6.37) kernel on an embedded plaform,
> but if I look at the current torvalds kernel (fbdev kernel looks
> rather old!?), the bug seems to still be there.
>
> When looking at the msync(2) man page, msync is meant to return 0 on
> success and -1 on failure, however we are seeing it return 1, which is
> undocumented and in my opinion wrong.
>
> This is because the fb_deferred_io_fsync method returns the code from
> 'schedule_delayed_work', which is either 0 or 1 depending on whether
> the work was already scheduled, leading to the possibility that msync
> returns the value 1 to userland.
Indeed, looks broken. schedule_delayed_work() never returns an error, it
just returns a bool telling if the work was already scheduled.
I guess no one really uses deferred io much =).
Here's a patch:
From 9cc80b3cf9d6023ede084d024ed8b5c0cb247bc2 Mon Sep 17 00:00:00 2001
From: Tomi Valkeinen <tomi.valkeinen@ti.com>
Date: Fri, 19 Dec 2014 13:55:41 +0200
Subject: [PATCH] video/fbdev: fix defio's fsync
fb_deferred_io_fsync() returns the value of schedule_delayed_work() as
an error code, but schedule_delayed_work() does not return an error. It
returns true/false depending on whether the work was already queued.
Fix this by ignoring the return value of schedule_delayed_work().
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
drivers/video/fbdev/core/fb_defio.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/video/fbdev/core/fb_defio.c
b/drivers/video/fbdev/core/fb_defio.c
index 900aa4ecd617..d6cab1fd9a47 100644
--- a/drivers/video/fbdev/core/fb_defio.c
+++ b/drivers/video/fbdev/core/fb_defio.c
@@ -83,9 +83,10 @@ int fb_deferred_io_fsync(struct file *file, loff_t
start, loff_t end, int datasy
cancel_delayed_work_sync(&info->deferred_work);
/* Run it immediately */
- err = schedule_delayed_work(&info->deferred_work, 0);
+ schedule_delayed_work(&info->deferred_work, 0);
mutex_unlock(&inode->i_mutex);
- return err;
+
+ return 0;
}
EXPORT_SYMBOL_GPL(fb_deferred_io_fsync);
--
2.2.0
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply related
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