Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* Re: [PATCH] Revert "backlight: pwm: Handle EPROBE_DEFER while requesting the PWM"
From: Thierry Reding @ 2015-10-05 14:07 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: Nicolas Ferre, Robert Jarzmik, linux-pwm, Alexandre Belloni,
	Jingoo Han, Lee Jones, Jean-Christophe Plagniol-Villard,
	Tomi Valkeinen, linux-fbdev, linux-kernel
In-Reply-To: <20151005153024.3a245b0b@bbrezillon>

[-- Attachment #1: Type: text/plain, Size: 6483 bytes --]

On Mon, Oct 05, 2015 at 03:30:24PM +0200, Boris Brezillon wrote:
> On Mon, 5 Oct 2015 14:58:03 +0200
> Thierry Reding <thierry.reding@gmail.com> wrote:
> 
> > On Mon, Oct 05, 2015 at 01:19:12PM +0200, Boris Brezillon wrote:
> > > Hi Thierry,
> > > 
> > > On Mon, 5 Oct 2015 11:35:43 +0200
> > > Thierry Reding <thierry.reding@gmail.com> wrote:
> > > 
> > > > On Thu, Oct 01, 2015 at 10:00:22AM +0200, Nicolas Ferre wrote:
> > > > > Le 30/09/2015 21:29, Robert Jarzmik a écrit :
> > > > > > Robert Jarzmik <robert.jarzmik@free.fr> writes:
> > > > > > 
> > > > > >> This reverts commit 68feaca0b13e453aa14ee064c1736202b48b342f.
> > > > > >> This commit breaks legacy platforms, for which :
> > > > > >>  (a) no pwm table is added (legacy platforms)
> > > > > >>  (b) in this case, in pwm_get(), pmw_lookup_list is empty, and therefore
> > > > > >>      chosen == NULL, and therefore pwm_get() returns NULL, and pwm_get()
> > > > > >>      returns -EPROBE_DEFER
> > > > > >>  (c) as a consequence, this code is unreachable in pwm_bl.c :
> > > > > >>      if (IS_ERR(pb->pwm)) {
> > > > > >> 	ret = PTR_ERR(pb->pwm);
> > > > > >>  	dev_info(&pdev->dev, "%s:%d(): %d\n", __func__, __LINE__, ret);
> > > > > >>  	if (ret == -EPROBE_DEFER)
> > > > > >>  		goto err_alloc;
> > > > > >>
> > > > > >>  	dev_err(&pdev->dev, "unable to request PWM, trying legacy API\n");
> > > > > >>  	pb->legacy = true;
> > > > > >>  	pb->pwm = pwm_request(data->pwm_id, "pwm-backlight");
> > > > > >>
> > > > > >> As this code is unreachable, all legacy platforms relying on pwm_id are
> > > > > >> broken, amongst which pxa have been tested as broken.
> > > > > >>
> > > > > >> Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
> > > > > > Thierry, would you have a look please ?
> > > > > > As I said before, all legacy platform relying on pwm_id are broken. I'd like to
> > > > > > be sure this lands in the next -rc series.
> > > > > 
> > > > > Well, as I answered on the linux-pwm mailing-list (I was not in copy) here:
> > > > > http://article.gmane.org/gmane.linux.pwm/2744
> > > > > I wonder if it's not easier to fix the platforms and add the pwm tables...
> > > > > 
> > > > > Otherwise, Boris proposed this fix:
> > > > > 8<-----------------------------------------------------------
> > > > > diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
> > > > > index eff379b..00483d4 100644
> > > > > --- a/drivers/video/backlight/pwm_bl.c
> > > > > +++ b/drivers/video/backlight/pwm_bl.c
> > > > > @@ -273,15 +273,15 @@ static int pwm_backlight_probe(struct platform_device *pdev)
> > > > >  	pb->pwm = devm_pwm_get(&pdev->dev, NULL);
> > > > >  	if (IS_ERR(pb->pwm)) {
> > > > >  		ret = PTR_ERR(pb->pwm);
> > > > > -		if (ret == -EPROBE_DEFER)
> > > > > -			goto err_alloc;
> > > > >  
> > > > >  		dev_err(&pdev->dev, "unable to request PWM, trying legacy API\n");
> > > > >  		pb->legacy = true;
> > > > >  		pb->pwm = pwm_request(data->pwm_id, "pwm-backlight");
> > > > >  		if (IS_ERR(pb->pwm)) {
> > > > >  			dev_err(&pdev->dev, "unable to request legacy PWM\n");
> > > > > -			ret = PTR_ERR(pb->pwm);
> > > > > +			if (ret != -EPROBE_DEFER)
> > > > > +				ret = PTR_ERR(pb->pwm);
> > > > > +
> > > > >  			goto err_alloc;
> > > > >  		}
> > > > >  	}
> > > > > 
> > > > > which is not tested and may add an extra non-valid error log.
> > > > 
> > > > This is a little risky in my opinion. Not only does it print two error
> > > > messages for non-legacy platforms (that would be another regression if
> > > > you want to be nit-picking), but it is subtly buggy. If you have a
> > > > system with multiple PWM providers, you could end up failing the first
> > > > pwm_get() with -EPROBE_DEFER but then continue to the legacy case, and
> > > > this could succeed because data->pwm_id == 0, and that other provider
> > > > could be exporting the PWM with this ID. If I remember correctly this
> > > > was one of the reasons why the offending commit was merged in the first
> > > > place.
> > > 
> > > Just for the record, when I proposed this fix to Nicolas, I clearly
> > > stated that this was not the way to go, and that fixing the offending
> > > platforms to use PWM lookup table was the only sane solution, though I
> > > didn't thought about the invalid PWM id case leading to buggy behavior.
> > 
> > As chance would have it, this bubbled to the top of my inbox today:
> > 
> > 	http://patchwork.ozlabs.org/patch/483993/
> 
> AFAICT, this is not valid either. This patch is assuming -EPROBE_DEFER
> can only be returned in the DT case, which is not the case: it is also
> returned if the PWMs were declared with a lookup table but the driver
> is not registered yet (module not loaded, or driver registration
> taking place after the PWM backlight driver).

Right, the non-DT, slightly less legacy case...

> If we were about to differentiate the missing PWM definition from
> the missing driver case, we should do something like this [1].
> 
> Best Regards,
> 
> Boris
> 
> [1]http://code.bulix.org/2oozbq-89125

Haha, I came up with exactly this earlier and I've been trying to think
of ways in which it could potentially break.

Thierry

--- >8 ---
From f7fee34e0c414b4268c59e97937c51e0c91a74cf Mon Sep 17 00:00:00 2001
From: Thierry Reding <thierry.reding@gmail.com>
Date: Mon, 5 Oct 2015 14:38:32 +0200
Subject: [PATCH] pwm: Return -ENODEV if no PWM lookup match is found

When looking up a PWM using the lookup table, assume that all entries
will have been added already, so failure to find a match means that no
corresponding entry has been registered.

This fixes an issue where -EPROBE_DEFER would be returned if the PWM
lookup table is empty. After this fix, -EPROBE_DEFER is reserved for
situations where no provider has yet registered for a matching entry.

Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
---
 drivers/pwm/core.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index 3f9df3ea3350..94e5af123660 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -719,8 +719,10 @@ struct pwm_device *pwm_get(struct device *dev, const char *con_id)
 		}
 	}
 
-	if (!chosen)
+	if (!chosen) {
+		pwm = ERR_PTR(-ENODEV);
 		goto out;
+	}
 
 	chip = pwmchip_find_by_name(chosen->provider);
 	if (!chip)
-- 
2.5.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply related

* Re: [PATCH] Revert "backlight: pwm: Handle EPROBE_DEFER while requesting the PWM"
From: Boris Brezillon @ 2015-10-05 15:25 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Nicolas Ferre, Robert Jarzmik, linux-pwm, Alexandre Belloni,
	Jingoo Han, Lee Jones, Jean-Christophe Plagniol-Villard,
	Tomi Valkeinen, linux-fbdev, linux-kernel
In-Reply-To: <20151005140741.GA10625@ulmo>

On Mon, 5 Oct 2015 16:07:41 +0200
Thierry Reding <thierry.reding@gmail.com> wrote:

> On Mon, Oct 05, 2015 at 03:30:24PM +0200, Boris Brezillon wrote:
> > On Mon, 5 Oct 2015 14:58:03 +0200
> > Thierry Reding <thierry.reding@gmail.com> wrote:
> > 
> > > On Mon, Oct 05, 2015 at 01:19:12PM +0200, Boris Brezillon wrote:
> > > > Hi Thierry,
> > > > 
> > > > On Mon, 5 Oct 2015 11:35:43 +0200
> > > > Thierry Reding <thierry.reding@gmail.com> wrote:
> > > > 
> > > > > On Thu, Oct 01, 2015 at 10:00:22AM +0200, Nicolas Ferre wrote:
> > > > > > Le 30/09/2015 21:29, Robert Jarzmik a écrit :
> > > > > > > Robert Jarzmik <robert.jarzmik@free.fr> writes:
> > > > > > > 
> > > > > > >> This reverts commit 68feaca0b13e453aa14ee064c1736202b48b342f.
> > > > > > >> This commit breaks legacy platforms, for which :
> > > > > > >>  (a) no pwm table is added (legacy platforms)
> > > > > > >>  (b) in this case, in pwm_get(), pmw_lookup_list is empty, and therefore
> > > > > > >>      chosen = NULL, and therefore pwm_get() returns NULL, and pwm_get()
> > > > > > >>      returns -EPROBE_DEFER
> > > > > > >>  (c) as a consequence, this code is unreachable in pwm_bl.c :
> > > > > > >>      if (IS_ERR(pb->pwm)) {
> > > > > > >> 	ret = PTR_ERR(pb->pwm);
> > > > > > >>  	dev_info(&pdev->dev, "%s:%d(): %d\n", __func__, __LINE__, ret);
> > > > > > >>  	if (ret = -EPROBE_DEFER)
> > > > > > >>  		goto err_alloc;
> > > > > > >>
> > > > > > >>  	dev_err(&pdev->dev, "unable to request PWM, trying legacy API\n");
> > > > > > >>  	pb->legacy = true;
> > > > > > >>  	pb->pwm = pwm_request(data->pwm_id, "pwm-backlight");
> > > > > > >>
> > > > > > >> As this code is unreachable, all legacy platforms relying on pwm_id are
> > > > > > >> broken, amongst which pxa have been tested as broken.
> > > > > > >>
> > > > > > >> Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
> > > > > > > Thierry, would you have a look please ?
> > > > > > > As I said before, all legacy platform relying on pwm_id are broken. I'd like to
> > > > > > > be sure this lands in the next -rc series.
> > > > > > 
> > > > > > Well, as I answered on the linux-pwm mailing-list (I was not in copy) here:
> > > > > > http://article.gmane.org/gmane.linux.pwm/2744
> > > > > > I wonder if it's not easier to fix the platforms and add the pwm tables...
> > > > > > 
> > > > > > Otherwise, Boris proposed this fix:
> > > > > > 8<-----------------------------------------------------------
> > > > > > diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
> > > > > > index eff379b..00483d4 100644
> > > > > > --- a/drivers/video/backlight/pwm_bl.c
> > > > > > +++ b/drivers/video/backlight/pwm_bl.c
> > > > > > @@ -273,15 +273,15 @@ static int pwm_backlight_probe(struct platform_device *pdev)
> > > > > >  	pb->pwm = devm_pwm_get(&pdev->dev, NULL);
> > > > > >  	if (IS_ERR(pb->pwm)) {
> > > > > >  		ret = PTR_ERR(pb->pwm);
> > > > > > -		if (ret = -EPROBE_DEFER)
> > > > > > -			goto err_alloc;
> > > > > >  
> > > > > >  		dev_err(&pdev->dev, "unable to request PWM, trying legacy API\n");
> > > > > >  		pb->legacy = true;
> > > > > >  		pb->pwm = pwm_request(data->pwm_id, "pwm-backlight");
> > > > > >  		if (IS_ERR(pb->pwm)) {
> > > > > >  			dev_err(&pdev->dev, "unable to request legacy PWM\n");
> > > > > > -			ret = PTR_ERR(pb->pwm);
> > > > > > +			if (ret != -EPROBE_DEFER)
> > > > > > +				ret = PTR_ERR(pb->pwm);
> > > > > > +
> > > > > >  			goto err_alloc;
> > > > > >  		}
> > > > > >  	}
> > > > > > 
> > > > > > which is not tested and may add an extra non-valid error log.
> > > > > 
> > > > > This is a little risky in my opinion. Not only does it print two error
> > > > > messages for non-legacy platforms (that would be another regression if
> > > > > you want to be nit-picking), but it is subtly buggy. If you have a
> > > > > system with multiple PWM providers, you could end up failing the first
> > > > > pwm_get() with -EPROBE_DEFER but then continue to the legacy case, and
> > > > > this could succeed because data->pwm_id = 0, and that other provider
> > > > > could be exporting the PWM with this ID. If I remember correctly this
> > > > > was one of the reasons why the offending commit was merged in the first
> > > > > place.
> > > > 
> > > > Just for the record, when I proposed this fix to Nicolas, I clearly
> > > > stated that this was not the way to go, and that fixing the offending
> > > > platforms to use PWM lookup table was the only sane solution, though I
> > > > didn't thought about the invalid PWM id case leading to buggy behavior.
> > > 
> > > As chance would have it, this bubbled to the top of my inbox today:
> > > 
> > > 	http://patchwork.ozlabs.org/patch/483993/
> > 
> > AFAICT, this is not valid either. This patch is assuming -EPROBE_DEFER
> > can only be returned in the DT case, which is not the case: it is also
> > returned if the PWMs were declared with a lookup table but the driver
> > is not registered yet (module not loaded, or driver registration
> > taking place after the PWM backlight driver).
> 
> Right, the non-DT, slightly less legacy case...
> 
> > If we were about to differentiate the missing PWM definition from
> > the missing driver case, we should do something like this [1].
> > 
> > Best Regards,
> > 
> > Boris
> > 
> > [1]http://code.bulix.org/2oozbq-89125
> 
> Haha, I came up with exactly this earlier and I've been trying to think
> of ways in which it could potentially break.

From a quick glance, I don't see any obvious problem in this approach.

> 
> Thierry
> 
> --- >8 ---
> From f7fee34e0c414b4268c59e97937c51e0c91a74cf Mon Sep 17 00:00:00 2001
> From: Thierry Reding <thierry.reding@gmail.com>
> Date: Mon, 5 Oct 2015 14:38:32 +0200
> Subject: [PATCH] pwm: Return -ENODEV if no PWM lookup match is found
> 
> When looking up a PWM using the lookup table, assume that all entries
> will have been added already, so failure to find a match means that no
> corresponding entry has been registered.
> 
> This fixes an issue where -EPROBE_DEFER would be returned if the PWM
> lookup table is empty. After this fix, -EPROBE_DEFER is reserved for
> situations where no provider has yet registered for a matching entry.
> 
> Signed-off-by: Thierry Reding <thierry.reding@gmail.com>

Not sure it has any value since I proposed the same patch, but here
is my

Reviewed-by: Boris Brezillon <boris.brezillon@free-electrons.com>

> ---
>  drivers/pwm/core.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
> index 3f9df3ea3350..94e5af123660 100644
> --- a/drivers/pwm/core.c
> +++ b/drivers/pwm/core.c
> @@ -719,8 +719,10 @@ struct pwm_device *pwm_get(struct device *dev, const char *con_id)
>  		}
>  	}
>  
> -	if (!chosen)
> +	if (!chosen) {
> +		pwm = ERR_PTR(-ENODEV);
>  		goto out;
> +	}
>  
>  	chip = pwmchip_find_by_name(chosen->provider);
>  	if (!chip)



-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply

* Re: [PATCH v2] video: fbdev: add Marvell PXA framebuffer binding
From: Philipp Zabel @ 2015-10-06  6:58 UTC (permalink / raw)
  To: Robert Jarzmik
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	devicetree-u79uwXL29TY76Z2rM5mHXA, LKML,
	Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1443954683-13126-1-git-send-email-robert.jarzmik-GANU6spQydw@public.gmane.org>

On Sun, Oct 4, 2015 at 12:31 PM, Robert Jarzmik <robert.jarzmik@free.fr> wrote:
> Add documentation for the PXA frambuffer devicetree binding.
>
> Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
> ---
> Since v1: Philipp's review on the whole binding
> ---
>  .../devicetree/bindings/video/marvell,pxafb.txt    | 80 ++++++++++++++++++++++
>  1 file changed, 80 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/video/marvell,pxafb.txt
>
> diff --git a/Documentation/devicetree/bindings/video/marvell,pxafb.txt b/Documentation/devicetree/bindings/video/marvell,pxafb.txt
> new file mode 100644
> index 000000000000..4d6bd490680d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/video/marvell,pxafb.txt
> @@ -0,0 +1,80 @@
> +PXA LCDC Framebuffer
> +--------------------
> +
> +Required properties:
> + - compatible :
> +       "marvell,pxa2xx-lcdc",
> + - reg : Should contain 1 register ranges(address and length).
> +        Can contain an additional register range(address and length)
> +        for fixed framebuffer memory. Useful for dedicated memories.
> + - interrupts : framebuffer controller interrupt
> +
> +Required nodes:
> + - clocks: phandle to input clocks.
> + - port: connection to the LCD panel (see video-interfaces.txt)
> +        This nodes must have its properties bus-width and remote-endpoint set.
> +        This should be in the board dts.
> +
> +Example:
> +
> +       lcd-controller@40500000 {
> +               compatible = "marvell,pxa2xx-lcdc";
> +               reg = <0x44000000 0x10000>;
> +               interrupts = <17>;
> +               clocks = <&clks CLK_LCD>;
> +               interrupts = <23>;

The interrupt line is duplicated in the example.

> +               status = "okay";
> +
> +               port {
> +                       lcdc_out: endpoint {
> +                               remote-endpoint = <&panel_in>;
> +                               bus-width = <16>;
> +                       };
> +               };
> +       };
> +
> +PXA LCDC Display
> +----------------
> +Required properties (as per of_videomode_helper):
> + - lcd-type: either "mono-stn", "mono-dstn", "color-stn", "color-dstn",
> +                   "color-tft", "smart-panel"

Would it make sense to make this property optional and have the
default be "color-tft"? That is probably the most common one by far.

regards
Philipp

^ permalink raw reply

* [PATCH 1/2] fb.h: Provide alternate screen_base pointer
From: Lars Svensson @ 2015-10-06  7:02 UTC (permalink / raw)
  To: thomas.petazzoni, noralf, plagnioj, tomi.valkeinen
  Cc: linux-fbdev, gregkh, devel, linux-kernel, dan.carpenter,
	Lars Svensson

Some drivers use member screen_base of struct fb_info to store non-
__iomem pointers, creating the need for ugly __force typecasts to
avoid sparse warnings. This adds an alternate pointer without the
__iomem qualifyer for this use.

Signed-off-by: Lars Svensson <lars1.svensson@sonymobile.com>
---
New version of my previous patch, reworked as suggested by Dan
Carpenter. For the name of the pointer, "screen_buffer" was the
best I could come up with. Other suggestions?

//Lars
---
 include/linux/fb.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/include/linux/fb.h b/include/linux/fb.h
index bc9afa7..41a3b11 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -483,7 +483,10 @@ struct fb_info {
 #ifdef CONFIG_FB_TILEBLITTING
 	struct fb_tile_ops *tileops;    /* Tile Blitting */
 #endif
-	char __iomem *screen_base;	/* Virtual address */
+	union {
+		char __iomem *screen_base;	/* Virtual address */
+		char *screen_buffer;
+	};
 	unsigned long screen_size;	/* Amount of ioremapped VRAM or 0 */ 
 	void *pseudo_palette;		/* Fake palette of 16 colors */ 
 #define FBINFO_STATE_RUNNING	0
-- 
2.4.2


^ permalink raw reply related

* [PATCH 2/2] staging: fbtft: fix sparse warning in call to vfree()
From: Lars Svensson @ 2015-10-06  7:02 UTC (permalink / raw)
  To: thomas.petazzoni, noralf, plagnioj, tomi.valkeinen
  Cc: linux-fbdev, gregkh, devel, linux-kernel, dan.carpenter,
	Lars Svensson
In-Reply-To: <1444114926-18000-1-git-send-email-lars1.svensson@sonymobile.com>

Member screen_base in struct fb_info is declared __iomem causing
a sparse warning as below when passed to vfree(). Instead use non-
__iomem pointer screen_buffer to avoid the warning.

fbtft-core.c:922:39: warning: incorrect type in argument 1 \
(different address spaces)
fbtft-core.c:922:39:    expected void const *addr
fbtft-core.c:922:39:    got char [noderef] <asn:2>*screen_base

Signed-off-by: Lars Svensson <lars1.svensson@sonymobile.com>
---
 drivers/staging/fbtft/fbtft-core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c
index 7f5fa3d..a5fade2 100644
--- a/drivers/staging/fbtft/fbtft-core.c
+++ b/drivers/staging/fbtft/fbtft-core.c
@@ -779,7 +779,7 @@ struct fb_info *fbtft_framebuffer_alloc(struct fbtft_display *display,
 	if (!info)
 		goto alloc_fail;
 
-	info->screen_base = (u8 __force __iomem *)vmem;
+	info->screen_buffer = vmem;
 	info->fbops = fbops;
 	info->fbdefio = fbdefio;
 
@@ -919,7 +919,7 @@ EXPORT_SYMBOL(fbtft_framebuffer_alloc);
 void fbtft_framebuffer_release(struct fb_info *info)
 {
 	fb_deferred_io_cleanup(info);
-	vfree(info->screen_base);
+	vfree(info->screen_buffer);
 	framebuffer_release(info);
 }
 EXPORT_SYMBOL(fbtft_framebuffer_release);
-- 
2.4.2


^ permalink raw reply related

* Re: [PATCH 2/2] staging: fbtft: fix sparse warning in call to vfree()
From: Dan Carpenter @ 2015-10-06  7:18 UTC (permalink / raw)
  To: Lars Svensson
  Cc: thomas.petazzoni, noralf, plagnioj, tomi.valkeinen, linux-fbdev,
	gregkh, devel, linux-kernel
In-Reply-To: <1444114926-18000-2-git-send-email-lars1.svensson@sonymobile.com>

Could you update everything in that directory all at once instead?

grep screen_base drivers/staging/fbtft/*c

regards,
dan carpenter


^ permalink raw reply

* Re: [PATCH v2 01/10] pwm: introduce default period and polarity concepts
From: Alexandre Belloni @ 2015-10-06 10:09 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1437406327-6207-2-git-send-email-boris.brezillon@free-electrons.com>

On 20/07/2015 at 17:31:58 +0200, Boris Brezillon wrote :
> When requested by a user, the PWM is assigned a default period and polarity
> extracted from the DT, the platform data or statically set by the driver.
> Those default values are currently stored in the period and polarity
> fields of the pwm_device struct, but they will be stored somewhere else
> once we have introduced the architecture allowing for hardware state
> retrieval.
> 
> The pwm_set_default_polarity and pwm_set_default_period should only be
> used by PWM drivers or the PWM core infrastructure to specify the
> default period and polarity values.
> 
> PWM users might call the pwm_get_default_period to query the default
> period value. There is currently no helper to query the default
> polarity, but it might be added later on if there is a need for it.
> 
> This patch also modifies all the places where the default helpers should
> be used in place of the standard ones.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
Acked-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply

* Re: [PATCH v2] video: fbdev: add Marvell PXA framebuffer binding
From: Rob Herring @ 2015-10-06 13:30 UTC (permalink / raw)
  To: Robert Jarzmik
  Cc: Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Philipp Zabel,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <1443954683-13126-1-git-send-email-robert.jarzmik-GANU6spQydw@public.gmane.org>

On Sun, Oct 4, 2015 at 5:31 AM, Robert Jarzmik <robert.jarzmik@free.fr> wrote:
> Add documentation for the PXA frambuffer devicetree binding.

Strictly speaking this is a binding for PXA display controller, not a
Linux FB driver. There are lots of "framebuffer" and "DRM" bindings
which I'm trying to curb.

> Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
> ---
> Since v1: Philipp's review on the whole binding
> ---
>  .../devicetree/bindings/video/marvell,pxafb.txt    | 80 ++++++++++++++++++++++

Please put in bindings/display/ as I'm consolidating all the display
related bindings there[1].

>  1 file changed, 80 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/video/marvell,pxafb.txt
>
> diff --git a/Documentation/devicetree/bindings/video/marvell,pxafb.txt b/Documentation/devicetree/bindings/video/marvell,pxafb.txt
> new file mode 100644
> index 000000000000..4d6bd490680d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/video/marvell,pxafb.txt
> @@ -0,0 +1,80 @@
> +PXA LCDC Framebuffer
> +--------------------
> +
> +Required properties:
> + - compatible :
> +       "marvell,pxa2xx-lcdc",

No differences in h/w for any of the chips?

> + - reg : Should contain 1 register ranges(address and length).
> +        Can contain an additional register range(address and length)
> +        for fixed framebuffer memory. Useful for dedicated memories.

This is memory that can't be used for anything else? We already have
reserved-memory for this if it is just RAM. There's also a binding for
on-chip SRAM which should probably be used if the memory is usable for
other things.

> + - interrupts : framebuffer controller interrupt
> +
> +Required nodes:
> + - clocks: phandle to input clocks.
> + - port: connection to the LCD panel (see video-interfaces.txt)
> +        This nodes must have its properties bus-width and remote-endpoint set.
> +        This should be in the board dts.
> +
> +Example:
> +
> +       lcd-controller@40500000 {
> +               compatible = "marvell,pxa2xx-lcdc";
> +               reg = <0x44000000 0x10000>;
> +               interrupts = <17>;
> +               clocks = <&clks CLK_LCD>;
> +               interrupts = <23>;
> +               status = "okay";
> +
> +               port {
> +                       lcdc_out: endpoint {
> +                               remote-endpoint = <&panel_in>;
> +                               bus-width = <16>;
> +                       };
> +               };
> +       };
> +
> +PXA LCDC Display

This should not be specific to PXA, but for this panel. This should be
in bindings/display/panel/.

> +----------------
> +Required properties (as per of_videomode_helper):
> + - lcd-type: either "mono-stn", "mono-dstn", "color-stn", "color-dstn",
> +                   "color-tft", "smart-panel"
> +
> +Optional properties (as per of_videomode_helper):
> + - power-supply: power supply regulator to the LCD to power it on or off
> +                (see regulator.txt)
> + - backlight: backlight control (see backlight.txt)
> +
> +Required nodes:
> + - port: connection to the LCD controller
> + - display-timings: panel timings (see display-timing.txt)

If lcd-type is smart-panel, then this node would not make sense.

> +
> +Example:
> +       panel {
> +               compatible = "toshiba,ltm0305a776";
> +               lcd-type = "color-tft";
> +
> +               power-supply = <&lcd_supply>;
> +               backlight = <&lcd_backlight>;
> +
> +               port {
> +                       panel_in: endpoint {
> +                               remote-endpoint = <&lcdc_out>;
> +                       };
> +               };
> +
> +               display-timings {
> +                       native-mode = <&timing0>;
> +                       timing0: 240p {
> +                               /* 240x320p24 */
> +                               clock-frequency = <4545000>;
> +                               hactive = <240>;
> +                               vactive = <320>;
> +                               hfront-porch = <4>;
> +                               hback-porch = <6>;
> +                               hsync-len = <4>;
> +                               vback-porch = <5>;
> +                               vfront-porch = <3>;
> +                               vsync-len = <2>;
> +                       };
> +               };
> +       };
> --
> 2.1.4
>

[1] https://lkml.org/lkml/2015/10/1/899

^ permalink raw reply

* [PATCH 3/3] video: fbdev: au1200fb: alloc mem from coherent pool/CMA
From: Manuel Lauss @ 2015-10-06 16:23 UTC (permalink / raw)
  To: Linux-MIPS; +Cc: Manuel Lauss, linux-fbdev
In-Reply-To: <1444148603-45454-1-git-send-email-manuel.lauss@gmail.com>

Change framebuffer memory allocation to grab some memory from the
coherent pool, which on MIPS causes the allocator to first try
to look for CMA-reserved memory.

Signed-off-by: Manuel Lauss <manuel.lauss@gmail.com>
Cc: linux-fbdev <linux-fbdev@vger.kernel.org>
---
Tested on Db1200 and Db1300 boards, with no issues

 drivers/video/fbdev/au1200fb.c | 13 +------------
 1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/drivers/video/fbdev/au1200fb.c b/drivers/video/fbdev/au1200fb.c
index f9507b1..0884197 100644
--- a/drivers/video/fbdev/au1200fb.c
+++ b/drivers/video/fbdev/au1200fb.c
@@ -1647,7 +1647,6 @@ static int au1200fb_drv_probe(struct platform_device *dev)
 	struct au1200fb_device *fbdev;
 	struct au1200fb_platdata *pd;
 	struct fb_info *fbi = NULL;
-	unsigned long page;
 	int bpp, plane, ret, irq;
 
 	print_info("" DRIVER_DESC "");
@@ -1693,7 +1692,7 @@ static int au1200fb_drv_probe(struct platform_device *dev)
 		/* Allocate the framebuffer to the maximum screen size */
 		fbdev->fb_len = (win->w[plane].xres * win->w[plane].yres * bpp) / 8;
 
-		fbdev->fb_mem = dmam_alloc_noncoherent(&dev->dev,
+		fbdev->fb_mem = dmam_alloc_coherent(&dev->dev,
 				PAGE_ALIGN(fbdev->fb_len),
 				&fbdev->fb_phys, GFP_KERNEL);
 		if (!fbdev->fb_mem) {
@@ -1702,16 +1701,6 @@ static int au1200fb_drv_probe(struct platform_device *dev)
 			return -ENOMEM;
 		}
 
-		/*
-		 * Set page reserved so that mmap will work. This is necessary
-		 * since we'll be remapping normal memory.
-		 */
-		for (page = (unsigned long)fbdev->fb_phys;
-		     page < PAGE_ALIGN((unsigned long)fbdev->fb_phys +
-			     fbdev->fb_len);
-		     page += PAGE_SIZE) {
-			SetPageReserved(pfn_to_page(page >> PAGE_SHIFT)); /* LCD DMA is NOT coherent on Au1200 */
-		}
 		print_dbg("Framebuffer memory map at %p", fbdev->fb_mem);
 		print_dbg("phys=0x%08x, size=%dK", fbdev->fb_phys, fbdev->fb_len / 1024);
 
-- 
2.5.3


^ permalink raw reply related

* Re: [PATCH v2] video: fbdev: add Marvell PXA framebuffer binding
From: Robert Jarzmik @ 2015-10-06 19:13 UTC (permalink / raw)
  To: Philipp Zabel
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	devicetree, LKML, Jean-Christophe Plagniol-Villard,
	Tomi Valkeinen, linux-fbdev
In-Reply-To: <CA+gwMcciuonC5Ektt3wdUcp7RW3E+caCuktYrhcYSqTrnVng9Q@mail.gmail.com>

Philipp Zabel <philipp.zabel@gmail.com> writes:

> On Sun, Oct 4, 2015 at 12:31 PM, Robert Jarzmik <robert.jarzmik@free.fr> wrote:
>> +       lcd-controller@40500000 {
>> +               compatible = "marvell,pxa2xx-lcdc";
>> +               reg = <0x44000000 0x10000>;
>> +               interrupts = <17>;
>> +               clocks = <&clks CLK_LCD>;
>> +               interrupts = <23>;
>
> The interrupt line is duplicated in the example.
Indeed, I'll fix that.
>
>> +               status = "okay";
>> +
>> +               port {
>> +                       lcdc_out: endpoint {
>> +                               remote-endpoint = <&panel_in>;
>> +                               bus-width = <16>;
>> +                       };
>> +               };
>> +       };
>> +
>> +PXA LCDC Display
>> +----------------
>> +Required properties (as per of_videomode_helper):
>> + - lcd-type: either "mono-stn", "mono-dstn", "color-stn", "color-dstn",
>> +                   "color-tft", "smart-panel"
>
> Would it make sense to make this property optional and have the
> default be "color-tft"? That is probably the most common one by far.
Yes, sure, let's do it that way for v3.

Cheers.

-- 
Robert

^ permalink raw reply

* Re: [PATCH v2] video: fbdev: add Marvell PXA framebuffer binding
From: Robert Jarzmik @ 2015-10-06 19:32 UTC (permalink / raw)
  To: Rob Herring
  Cc: Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Philipp Zabel,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
	linux-fbdev@vger.kernel.org
In-Reply-To: <CAL_JsqLy-pK4VpBkj1-wVg85XvPemssM-=Ckx=yDjiXKXbtwtA@mail.gmail.com>

Rob Herring <robh@kernel.org> writes:

> On Sun, Oct 4, 2015 at 5:31 AM, Robert Jarzmik <robert.jarzmik@free.fr> wrote:
>> Add documentation for the PXA frambuffer devicetree binding.
>
> Strictly speaking this is a binding for PXA display controller, not a
> Linux FB driver. There are lots of "framebuffer" and "DRM" bindings
> which I'm trying to curb.
Yes, that's very true. That deserves a new commit message and a new file name
(marvell,pxa2xx-lcd).
>>  .../devicetree/bindings/video/marvell,pxafb.txt    | 80 ++++++++++++++++++++++
>
> Please put in bindings/display/ as I'm consolidating all the display
> related bindings there[1].
Of course, for v3.

>> +++ b/Documentation/devicetree/bindings/video/marvell,pxafb.txt
>> @@ -0,0 +1,80 @@
>> +PXA LCDC Framebuffer
>> +--------------------
>> +
>> +Required properties:
>> + - compatible :
>> +       "marvell,pxa2xx-lcdc",
>
> No differences in h/w for any of the chips?
All pxa25x, pxa27x and pxa3xx are compatible.
AFAIK, pxa3xx has an IP with additional registers. But :
 - these (this) register(s) is not necessary for the display controller to work
   (it's more a control to shift red/green/blue values, and energy management)
 - all the registers in pxa2{5,7}x are the same in pxa3xx

The pxafb driver acts today on the subset of registers which are the same across
all pxaXXX variants. This is what made me think only one compatible property was
required.

If I'm wrong, I could add "marvell,pxa3xx-lcdc", is that what you think I should
do ?
>
>> + - reg : Should contain 1 register ranges(address and length).
>> +        Can contain an additional register range(address and length)
>> +        for fixed framebuffer memory. Useful for dedicated memories.
>
> This is memory that can't be used for anything else? We already have
> reserved-memory for this if it is just RAM. There's also a binding for
> on-chip SRAM which should probably be used if the memory is usable for
> other things.
That is a wrong copy paste I made. I was more thinking of having only 1 register
range, and no video memory reservation ...

>> +PXA LCDC Display
>
> This should not be specific to PXA, but for this panel. This should be
> in bindings/display/panel/.
Sure, for v3.

>> +----------------
>> +Required properties (as per of_videomode_helper):
>> + - lcd-type: either "mono-stn", "mono-dstn", "color-stn", "color-dstn",
>> +                   "color-tft", "smart-panel"
>> +
>> +Optional properties (as per of_videomode_helper):
>> + - power-supply: power supply regulator to the LCD to power it on or off
>> +                (see regulator.txt)
>> + - backlight: backlight control (see backlight.txt)
>> +
>> +Required nodes:
>> + - port: connection to the LCD controller
>> + - display-timings: panel timings (see display-timing.txt)
>
> If lcd-type is smart-panel, then this node would not make sense.
Ah I see, so this could be optional maybe ?

Actually when I'll move the panel definition to bindings/display/panel, is this
what I should do :
 - create a file marvell,pxa2xx-panel
 - input all these properties into this file

And then, when a board maintainer will create a devicetree description, he will
write something like :
      compatible = "toshiba,ltm0305a776";
      compatible = "marvell,pxa2xx-panel";
      lcd-type = "color-tft";
      ...

If that's the case, I wonder how to "enforce" that a panel used with
marvell,pxa2xx-lcdc (through the of_graph 'port' node) be compatible with
marvell,pxa2xx-panel ?

Cheers.

-- 
Robert

^ permalink raw reply

* [PATCH v2 1/2] fb.h: Provide alternate screen_base pointer
From: Lars Svensson @ 2015-10-07  5:45 UTC (permalink / raw)
  To: thomas.petazzoni, noralf, plagnioj, tomi.valkeinen
  Cc: linux-fbdev, gregkh, devel, linux-kernel, dan.carpenter,
	Lars Svensson

Some drivers use member screen_base of struct fb_info to store non-
__iomem pointers, creating the need for ugly __force typecasts to
avoid sparse warnings. This adds an alternate pointer without the
__iomem qualifyer for this use.

Signed-off-by: Lars Svensson <lars1.svensson@sonymobile.com>
---
 include/linux/fb.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/include/linux/fb.h b/include/linux/fb.h
index bc9afa7..41a3b11 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -483,7 +483,10 @@ struct fb_info {
 #ifdef CONFIG_FB_TILEBLITTING
 	struct fb_tile_ops *tileops;    /* Tile Blitting */
 #endif
-	char __iomem *screen_base;	/* Virtual address */
+	union {
+		char __iomem *screen_base;	/* Virtual address */
+		char *screen_buffer;
+	};
 	unsigned long screen_size;	/* Amount of ioremapped VRAM or 0 */ 
 	void *pseudo_palette;		/* Fake palette of 16 colors */ 
 #define FBINFO_STATE_RUNNING	0
-- 
2.4.2


^ permalink raw reply related

* [PATCH v2 2/2] staging: fbtft: use alternate screen pointer
From: Lars Svensson @ 2015-10-07  5:45 UTC (permalink / raw)
  To: thomas.petazzoni, noralf, plagnioj, tomi.valkeinen
  Cc: linux-fbdev, gregkh, devel, linux-kernel, dan.carpenter,
	Lars Svensson
In-Reply-To: <1444196732-27841-1-git-send-email-lars1.svensson@sonymobile.com>

Member screen_base in struct fb_info is declared with __iomem
qualifier causing sparse warnings when used as a regular ponter.
To avoid the warnings, instead use alternate non-__iomem pointer,
screen_buffer, troughout the driver.

Signed-off-by: Lars Svensson <lars1.svensson@sonymobile.com>
---
v2: removed all references to screen_base.
---
 drivers/staging/fbtft/fb_agm1264k-fl.c |  2 +-
 drivers/staging/fbtft/fb_pcd8544.c     |  2 +-
 drivers/staging/fbtft/fb_ra8875.c      |  2 +-
 drivers/staging/fbtft/fb_ssd1306.c     |  2 +-
 drivers/staging/fbtft/fb_tls8204.c     |  2 +-
 drivers/staging/fbtft/fb_uc1611.c      |  2 +-
 drivers/staging/fbtft/fb_uc1701.c      |  2 +-
 drivers/staging/fbtft/fb_watterott.c   |  4 ++--
 drivers/staging/fbtft/fbtft-bus.c      | 14 +++++++-------
 drivers/staging/fbtft/fbtft-core.c     |  4 ++--
 10 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/drivers/staging/fbtft/fb_agm1264k-fl.c b/drivers/staging/fbtft/fb_agm1264k-fl.c
index 94dd49c..5af372b 100644
--- a/drivers/staging/fbtft/fb_agm1264k-fl.c
+++ b/drivers/staging/fbtft/fb_agm1264k-fl.c
@@ -273,7 +273,7 @@ construct_line_bitmap(struct fbtft_par *par, u8 *dest, signed short *src,
 
 static int write_vmem(struct fbtft_par *par, size_t offset, size_t len)
 {
-	u16 *vmem16 = (u16 *)par->info->screen_base;
+	u16 *vmem16 = (u16 *)par->info->screen_buffer;
 	u8 *buf = par->txbuf.buf;
 	int x, y;
 	int ret = 0;
diff --git a/drivers/staging/fbtft/fb_pcd8544.c b/drivers/staging/fbtft/fb_pcd8544.c
index 15da0ec..27d6681 100644
--- a/drivers/staging/fbtft/fb_pcd8544.c
+++ b/drivers/staging/fbtft/fb_pcd8544.c
@@ -122,7 +122,7 @@ static void set_addr_win(struct fbtft_par *par, int xs, int ys, int xe, int ye)
 
 static int write_vmem(struct fbtft_par *par, size_t offset, size_t len)
 {
-	u16 *vmem16 = (u16 *)par->info->screen_base;
+	u16 *vmem16 = (u16 *)par->info->screen_buffer;
 	u8 *buf = par->txbuf.buf;
 	int x, y, i;
 	int ret = 0;
diff --git a/drivers/staging/fbtft/fb_ra8875.c b/drivers/staging/fbtft/fb_ra8875.c
index 54bc566..e8b2178 100644
--- a/drivers/staging/fbtft/fb_ra8875.c
+++ b/drivers/staging/fbtft/fb_ra8875.c
@@ -288,7 +288,7 @@ static int write_vmem16_bus8(struct fbtft_par *par, size_t offset, size_t len)
 		__func__, offset, len);
 
 	remain = len / 2;
-	vmem16 = (u16 *)(par->info->screen_base + offset);
+	vmem16 = (u16 *)(par->info->screen_buffer + offset);
 	tx_array_size = par->txbuf.len / 2;
 		txbuf16 = (u16 *)(par->txbuf.buf + 1);
 		tx_array_size -= 2;
diff --git a/drivers/staging/fbtft/fb_ssd1306.c b/drivers/staging/fbtft/fb_ssd1306.c
index 15ee44d..913b0f7 100644
--- a/drivers/staging/fbtft/fb_ssd1306.c
+++ b/drivers/staging/fbtft/fb_ssd1306.c
@@ -172,7 +172,7 @@ static int set_gamma(struct fbtft_par *par, unsigned long *curves)
 
 static int write_vmem(struct fbtft_par *par, size_t offset, size_t len)
 {
-	u16 *vmem16 = (u16 *)par->info->screen_base;
+	u16 *vmem16 = (u16 *)par->info->screen_buffer;
 	u8 *buf = par->txbuf.buf;
 	int x, y, i;
 	int ret = 0;
diff --git a/drivers/staging/fbtft/fb_tls8204.c b/drivers/staging/fbtft/fb_tls8204.c
index 3253a25..b66dc7f 100644
--- a/drivers/staging/fbtft/fb_tls8204.c
+++ b/drivers/staging/fbtft/fb_tls8204.c
@@ -104,7 +104,7 @@ static void set_addr_win(struct fbtft_par *par, int xs, int ys, int xe, int ye)
 
 static int write_vmem(struct fbtft_par *par, size_t offset, size_t len)
 {
-	u16 *vmem16 = (u16 *)par->info->screen_base;
+	u16 *vmem16 = (u16 *)par->info->screen_buffer;
 	int x, y, i;
 	int ret = 0;
 
diff --git a/drivers/staging/fbtft/fb_uc1611.c b/drivers/staging/fbtft/fb_uc1611.c
index 5cafa50..689a56a 100644
--- a/drivers/staging/fbtft/fb_uc1611.c
+++ b/drivers/staging/fbtft/fb_uc1611.c
@@ -229,7 +229,7 @@ static int set_var(struct fbtft_par *par)
 
 static int write_vmem(struct fbtft_par *par, size_t offset, size_t len)
 {
-	u8 *vmem8 = (u8 *)(par->info->screen_base);
+	u8 *vmem8 = (u8 *)(par->info->screen_buffer);
 	u8 *buf8 = (u8 *)(par->txbuf.buf);
 	u16 *buf16 = (u16 *)(par->txbuf.buf);
 	int line_length = par->info->fix.line_length;
diff --git a/drivers/staging/fbtft/fb_uc1701.c b/drivers/staging/fbtft/fb_uc1701.c
index 26d669b..96b7298 100644
--- a/drivers/staging/fbtft/fb_uc1701.c
+++ b/drivers/staging/fbtft/fb_uc1701.c
@@ -150,7 +150,7 @@ static void set_addr_win(struct fbtft_par *par, int xs, int ys, int xe, int ye)
 
 static int write_vmem(struct fbtft_par *par, size_t offset, size_t len)
 {
-	u16 *vmem16 = (u16 *)par->info->screen_base;
+	u16 *vmem16 = (u16 *)par->info->screen_buffer;
 	u8 *buf = par->txbuf.buf;
 	int x, y, i;
 	int ret = 0;
diff --git a/drivers/staging/fbtft/fb_watterott.c b/drivers/staging/fbtft/fb_watterott.c
index 8eae6ef..ed88e2d 100644
--- a/drivers/staging/fbtft/fb_watterott.c
+++ b/drivers/staging/fbtft/fb_watterott.c
@@ -73,7 +73,7 @@ static void write_reg8_bus8(struct fbtft_par *par, int len, ...)
 static int write_vmem(struct fbtft_par *par, size_t offset, size_t len)
 {
 	unsigned start_line, end_line;
-	u16 *vmem16 = (u16 *)(par->info->screen_base + offset);
+	u16 *vmem16 = (u16 *)(par->info->screen_buffer + offset);
 	u16 *pos = par->txbuf.buf + 1;
 	u16 *buf16 = par->txbuf.buf + 10;
 	int i, j;
@@ -112,7 +112,7 @@ static int write_vmem(struct fbtft_par *par, size_t offset, size_t len)
 static int write_vmem_8bit(struct fbtft_par *par, size_t offset, size_t len)
 {
 	unsigned start_line, end_line;
-	u16 *vmem16 = (u16 *)(par->info->screen_base + offset);
+	u16 *vmem16 = (u16 *)(par->info->screen_buffer + offset);
 	u16 *pos = par->txbuf.buf + 1;
 	u8 *buf8 = par->txbuf.buf + 10;
 	int i, j;
diff --git a/drivers/staging/fbtft/fbtft-bus.c b/drivers/staging/fbtft/fbtft-bus.c
index 912c632..04efe90 100644
--- a/drivers/staging/fbtft/fbtft-bus.c
+++ b/drivers/staging/fbtft/fbtft-bus.c
@@ -142,7 +142,7 @@ int fbtft_write_vmem16_bus8(struct fbtft_par *par, size_t offset, size_t len)
 		__func__, offset, len);
 
 	remain = len / 2;
-	vmem16 = (u16 *)(par->info->screen_base + offset);
+	vmem16 = (u16 *)(par->info->screen_buffer + offset);
 
 	if (par->gpio.dc != -1)
 		gpio_set_value(par->gpio.dc, 1);
@@ -184,7 +184,7 @@ EXPORT_SYMBOL(fbtft_write_vmem16_bus8);
 /* 16 bit pixel over 9-bit SPI bus: dc + high byte, dc + low byte */
 int fbtft_write_vmem16_bus9(struct fbtft_par *par, size_t offset, size_t len)
 {
-	u8 __iomem *vmem8;
+	u8 *vmem8;
 	u16 *txbuf16 = par->txbuf.buf;
 	size_t remain;
 	size_t to_copy;
@@ -201,7 +201,7 @@ int fbtft_write_vmem16_bus9(struct fbtft_par *par, size_t offset, size_t len)
 	}
 
 	remain = len;
-	vmem8 = par->info->screen_base + offset;
+	vmem8 = par->info->screen_buffer + offset;
 
 	tx_array_size = par->txbuf.len / 2;
 
@@ -212,12 +212,12 @@ int fbtft_write_vmem16_bus9(struct fbtft_par *par, size_t offset, size_t len)
 
 #ifdef __LITTLE_ENDIAN
 		for (i = 0; i < to_copy; i += 2) {
-			txbuf16[i]     = 0x0100 | ioread8(vmem8 + i + 1);
-			txbuf16[i + 1] = 0x0100 | ioread8(vmem8 + i);
+			txbuf16[i]     = 0x0100 | vmem8[i + 1];
+			txbuf16[i + 1] = 0x0100 | vmem8[i];
 		}
 #else
 		for (i = 0; i < to_copy; i++)
-			txbuf16[i]   = 0x0100 | ioread8(vmem8 + i);
+			txbuf16[i]   = 0x0100 | vmem8[i];
 #endif
 		vmem8 = vmem8 + to_copy;
 		ret = par->fbtftops.write(par, par->txbuf.buf, to_copy*2);
@@ -245,7 +245,7 @@ int fbtft_write_vmem16_bus16(struct fbtft_par *par, size_t offset, size_t len)
 	fbtft_par_dbg(DEBUG_WRITE_VMEM, par, "%s(offset=%zu, len=%zu)\n",
 		__func__, offset, len);
 
-	vmem16 = (u16 *)(par->info->screen_base + offset);
+	vmem16 = (u16 *)(par->info->screen_buffer + offset);
 
 	if (par->gpio.dc != -1)
 		gpio_set_value(par->gpio.dc, 1);
diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c
index 7f5fa3d..a5fade2 100644
--- a/drivers/staging/fbtft/fbtft-core.c
+++ b/drivers/staging/fbtft/fbtft-core.c
@@ -779,7 +779,7 @@ struct fb_info *fbtft_framebuffer_alloc(struct fbtft_display *display,
 	if (!info)
 		goto alloc_fail;
 
-	info->screen_base = (u8 __force __iomem *)vmem;
+	info->screen_buffer = vmem;
 	info->fbops = fbops;
 	info->fbdefio = fbdefio;
 
@@ -919,7 +919,7 @@ EXPORT_SYMBOL(fbtft_framebuffer_alloc);
 void fbtft_framebuffer_release(struct fb_info *info)
 {
 	fb_deferred_io_cleanup(info);
-	vfree(info->screen_base);
+	vfree(info->screen_buffer);
 	framebuffer_release(info);
 }
 EXPORT_SYMBOL(fbtft_framebuffer_release);
-- 
2.4.2


^ permalink raw reply related

* Re: [PATCH v2 2/2] staging: fbtft: use alternate screen pointer
From: Dan Carpenter @ 2015-10-07  6:04 UTC (permalink / raw)
  To: Lars Svensson
  Cc: thomas.petazzoni, noralf, plagnioj, tomi.valkeinen, linux-fbdev,
	gregkh, devel, linux-kernel
In-Reply-To: <1444196732-27841-2-git-send-email-lars1.svensson@sonymobile.com>

The change to fbtft_write_vmem16_bus9() is a bugfix, which is great, but
it's not mentioned in the changelog.  Really it should be a separate
commit.

Fixes: 02eb884f4e26 ('drivers: staging: fbtft: fbtft-bus.c: Fix different address space warning on I/O mem')

So it looks like this:
[patch 1] fb.h: Provide alternate screen_base pointer
[patch 2] sed /screen_base/screen_buffer/
[patch 3] fix a bug in fbtft_write_vmem16_bus9()

regards,
dan carpenter


^ permalink raw reply

* [PATCH v3 1/3] fb.h: Provide alternate screen_base pointer
From: Lars Svensson @ 2015-10-07  7:20 UTC (permalink / raw)
  To: thomas.petazzoni, noralf, plagnioj, tomi.valkeinen
  Cc: linux-fbdev, gregkh, devel, linux-kernel, dan.carpenter,
	Lars Svensson

Some drivers use member screen_base of struct fb_info to store non-
__iomem pointers, creating the need for ugly __force typecasts to
avoid sparse warnings. This adds an alternate pointer without the
__iomem qualifyer for this use.

Signed-off-by: Lars Svensson <lars1.svensson@sonymobile.com>
---
Patch v3: bugfix in fbtft-bus.c:fbtft_write_vmem16_bus9() in
a separate commit.
---
 include/linux/fb.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/include/linux/fb.h b/include/linux/fb.h
index bc9afa7..41a3b11 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -483,7 +483,10 @@ struct fb_info {
 #ifdef CONFIG_FB_TILEBLITTING
 	struct fb_tile_ops *tileops;    /* Tile Blitting */
 #endif
-	char __iomem *screen_base;	/* Virtual address */
+	union {
+		char __iomem *screen_base;	/* Virtual address */
+		char *screen_buffer;
+	};
 	unsigned long screen_size;	/* Amount of ioremapped VRAM or 0 */ 
 	void *pseudo_palette;		/* Fake palette of 16 colors */ 
 #define FBINFO_STATE_RUNNING	0
-- 
2.4.2


^ permalink raw reply related

* [PATCH v3 2/3] staging: fbtft: use alternate screen pointer
From: Lars Svensson @ 2015-10-07  7:20 UTC (permalink / raw)
  To: thomas.petazzoni, noralf, plagnioj, tomi.valkeinen
  Cc: linux-fbdev, gregkh, devel, linux-kernel, dan.carpenter,
	Lars Svensson
In-Reply-To: <1444202414-21271-1-git-send-email-lars1.svensson@sonymobile.com>

Member screen_base in struct fb_info is declared with __iomem
qualifier causing sparse warnings when used as a regular ponter.
To avoid the warnings, instead use alternate non-__iomem pointer,
screen_buffer, troughout the driver.

Signed-off-by: Lars Svensson <lars1.svensson@sonymobile.com>
---
 drivers/staging/fbtft/fb_agm1264k-fl.c | 2 +-
 drivers/staging/fbtft/fb_pcd8544.c     | 2 +-
 drivers/staging/fbtft/fb_ra8875.c      | 2 +-
 drivers/staging/fbtft/fb_ssd1306.c     | 2 +-
 drivers/staging/fbtft/fb_tls8204.c     | 2 +-
 drivers/staging/fbtft/fb_uc1611.c      | 2 +-
 drivers/staging/fbtft/fb_uc1701.c      | 2 +-
 drivers/staging/fbtft/fb_watterott.c   | 4 ++--
 drivers/staging/fbtft/fbtft-bus.c      | 6 +++---
 drivers/staging/fbtft/fbtft-core.c     | 4 ++--
 10 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/fbtft/fb_agm1264k-fl.c b/drivers/staging/fbtft/fb_agm1264k-fl.c
index 94dd49c..5af372b 100644
--- a/drivers/staging/fbtft/fb_agm1264k-fl.c
+++ b/drivers/staging/fbtft/fb_agm1264k-fl.c
@@ -273,7 +273,7 @@ construct_line_bitmap(struct fbtft_par *par, u8 *dest, signed short *src,
 
 static int write_vmem(struct fbtft_par *par, size_t offset, size_t len)
 {
-	u16 *vmem16 = (u16 *)par->info->screen_base;
+	u16 *vmem16 = (u16 *)par->info->screen_buffer;
 	u8 *buf = par->txbuf.buf;
 	int x, y;
 	int ret = 0;
diff --git a/drivers/staging/fbtft/fb_pcd8544.c b/drivers/staging/fbtft/fb_pcd8544.c
index 15da0ec..27d6681 100644
--- a/drivers/staging/fbtft/fb_pcd8544.c
+++ b/drivers/staging/fbtft/fb_pcd8544.c
@@ -122,7 +122,7 @@ static void set_addr_win(struct fbtft_par *par, int xs, int ys, int xe, int ye)
 
 static int write_vmem(struct fbtft_par *par, size_t offset, size_t len)
 {
-	u16 *vmem16 = (u16 *)par->info->screen_base;
+	u16 *vmem16 = (u16 *)par->info->screen_buffer;
 	u8 *buf = par->txbuf.buf;
 	int x, y, i;
 	int ret = 0;
diff --git a/drivers/staging/fbtft/fb_ra8875.c b/drivers/staging/fbtft/fb_ra8875.c
index 54bc566..e8b2178 100644
--- a/drivers/staging/fbtft/fb_ra8875.c
+++ b/drivers/staging/fbtft/fb_ra8875.c
@@ -288,7 +288,7 @@ static int write_vmem16_bus8(struct fbtft_par *par, size_t offset, size_t len)
 		__func__, offset, len);
 
 	remain = len / 2;
-	vmem16 = (u16 *)(par->info->screen_base + offset);
+	vmem16 = (u16 *)(par->info->screen_buffer + offset);
 	tx_array_size = par->txbuf.len / 2;
 		txbuf16 = (u16 *)(par->txbuf.buf + 1);
 		tx_array_size -= 2;
diff --git a/drivers/staging/fbtft/fb_ssd1306.c b/drivers/staging/fbtft/fb_ssd1306.c
index 15ee44d..913b0f7 100644
--- a/drivers/staging/fbtft/fb_ssd1306.c
+++ b/drivers/staging/fbtft/fb_ssd1306.c
@@ -172,7 +172,7 @@ static int set_gamma(struct fbtft_par *par, unsigned long *curves)
 
 static int write_vmem(struct fbtft_par *par, size_t offset, size_t len)
 {
-	u16 *vmem16 = (u16 *)par->info->screen_base;
+	u16 *vmem16 = (u16 *)par->info->screen_buffer;
 	u8 *buf = par->txbuf.buf;
 	int x, y, i;
 	int ret = 0;
diff --git a/drivers/staging/fbtft/fb_tls8204.c b/drivers/staging/fbtft/fb_tls8204.c
index 3253a25..b66dc7f 100644
--- a/drivers/staging/fbtft/fb_tls8204.c
+++ b/drivers/staging/fbtft/fb_tls8204.c
@@ -104,7 +104,7 @@ static void set_addr_win(struct fbtft_par *par, int xs, int ys, int xe, int ye)
 
 static int write_vmem(struct fbtft_par *par, size_t offset, size_t len)
 {
-	u16 *vmem16 = (u16 *)par->info->screen_base;
+	u16 *vmem16 = (u16 *)par->info->screen_buffer;
 	int x, y, i;
 	int ret = 0;
 
diff --git a/drivers/staging/fbtft/fb_uc1611.c b/drivers/staging/fbtft/fb_uc1611.c
index 5cafa50..689a56a 100644
--- a/drivers/staging/fbtft/fb_uc1611.c
+++ b/drivers/staging/fbtft/fb_uc1611.c
@@ -229,7 +229,7 @@ static int set_var(struct fbtft_par *par)
 
 static int write_vmem(struct fbtft_par *par, size_t offset, size_t len)
 {
-	u8 *vmem8 = (u8 *)(par->info->screen_base);
+	u8 *vmem8 = (u8 *)(par->info->screen_buffer);
 	u8 *buf8 = (u8 *)(par->txbuf.buf);
 	u16 *buf16 = (u16 *)(par->txbuf.buf);
 	int line_length = par->info->fix.line_length;
diff --git a/drivers/staging/fbtft/fb_uc1701.c b/drivers/staging/fbtft/fb_uc1701.c
index 26d669b..96b7298 100644
--- a/drivers/staging/fbtft/fb_uc1701.c
+++ b/drivers/staging/fbtft/fb_uc1701.c
@@ -150,7 +150,7 @@ static void set_addr_win(struct fbtft_par *par, int xs, int ys, int xe, int ye)
 
 static int write_vmem(struct fbtft_par *par, size_t offset, size_t len)
 {
-	u16 *vmem16 = (u16 *)par->info->screen_base;
+	u16 *vmem16 = (u16 *)par->info->screen_buffer;
 	u8 *buf = par->txbuf.buf;
 	int x, y, i;
 	int ret = 0;
diff --git a/drivers/staging/fbtft/fb_watterott.c b/drivers/staging/fbtft/fb_watterott.c
index 8eae6ef..ed88e2d 100644
--- a/drivers/staging/fbtft/fb_watterott.c
+++ b/drivers/staging/fbtft/fb_watterott.c
@@ -73,7 +73,7 @@ static void write_reg8_bus8(struct fbtft_par *par, int len, ...)
 static int write_vmem(struct fbtft_par *par, size_t offset, size_t len)
 {
 	unsigned start_line, end_line;
-	u16 *vmem16 = (u16 *)(par->info->screen_base + offset);
+	u16 *vmem16 = (u16 *)(par->info->screen_buffer + offset);
 	u16 *pos = par->txbuf.buf + 1;
 	u16 *buf16 = par->txbuf.buf + 10;
 	int i, j;
@@ -112,7 +112,7 @@ static int write_vmem(struct fbtft_par *par, size_t offset, size_t len)
 static int write_vmem_8bit(struct fbtft_par *par, size_t offset, size_t len)
 {
 	unsigned start_line, end_line;
-	u16 *vmem16 = (u16 *)(par->info->screen_base + offset);
+	u16 *vmem16 = (u16 *)(par->info->screen_buffer + offset);
 	u16 *pos = par->txbuf.buf + 1;
 	u8 *buf8 = par->txbuf.buf + 10;
 	int i, j;
diff --git a/drivers/staging/fbtft/fbtft-bus.c b/drivers/staging/fbtft/fbtft-bus.c
index 912c632..f5a36a4 100644
--- a/drivers/staging/fbtft/fbtft-bus.c
+++ b/drivers/staging/fbtft/fbtft-bus.c
@@ -142,7 +142,7 @@ int fbtft_write_vmem16_bus8(struct fbtft_par *par, size_t offset, size_t len)
 		__func__, offset, len);
 
 	remain = len / 2;
-	vmem16 = (u16 *)(par->info->screen_base + offset);
+	vmem16 = (u16 *)(par->info->screen_buffer + offset);
 
 	if (par->gpio.dc != -1)
 		gpio_set_value(par->gpio.dc, 1);
@@ -201,7 +201,7 @@ int fbtft_write_vmem16_bus9(struct fbtft_par *par, size_t offset, size_t len)
 	}
 
 	remain = len;
-	vmem8 = par->info->screen_base + offset;
+	vmem8 = par->info->screen_buffer + offset;
 
 	tx_array_size = par->txbuf.len / 2;
 
@@ -245,7 +245,7 @@ int fbtft_write_vmem16_bus16(struct fbtft_par *par, size_t offset, size_t len)
 	fbtft_par_dbg(DEBUG_WRITE_VMEM, par, "%s(offset=%zu, len=%zu)\n",
 		__func__, offset, len);
 
-	vmem16 = (u16 *)(par->info->screen_base + offset);
+	vmem16 = (u16 *)(par->info->screen_buffer + offset);
 
 	if (par->gpio.dc != -1)
 		gpio_set_value(par->gpio.dc, 1);
diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c
index 7f5fa3d..a5fade2 100644
--- a/drivers/staging/fbtft/fbtft-core.c
+++ b/drivers/staging/fbtft/fbtft-core.c
@@ -779,7 +779,7 @@ struct fb_info *fbtft_framebuffer_alloc(struct fbtft_display *display,
 	if (!info)
 		goto alloc_fail;
 
-	info->screen_base = (u8 __force __iomem *)vmem;
+	info->screen_buffer = vmem;
 	info->fbops = fbops;
 	info->fbdefio = fbdefio;
 
@@ -919,7 +919,7 @@ EXPORT_SYMBOL(fbtft_framebuffer_alloc);
 void fbtft_framebuffer_release(struct fb_info *info)
 {
 	fb_deferred_io_cleanup(info);
-	vfree(info->screen_base);
+	vfree(info->screen_buffer);
 	framebuffer_release(info);
 }
 EXPORT_SYMBOL(fbtft_framebuffer_release);
-- 
2.4.2


^ permalink raw reply related

* [PATCH v3 3/3] staging: fbtft: access screen buffer directly
From: Lars Svensson @ 2015-10-07  7:20 UTC (permalink / raw)
  To: thomas.petazzoni, noralf, plagnioj, tomi.valkeinen
  Cc: linux-fbdev, gregkh, devel, linux-kernel, dan.carpenter,
	Lars Svensson
In-Reply-To: <1444202414-21271-1-git-send-email-lars1.svensson@sonymobile.com>

In fbtft-bus.c:fbtft_write_vmem16_bus9(), ioread8() is used for
accessing the provided screen array. Since screen_buffer actually
points to an ordinary buffer, instead access it directly.

Signed-off-by: Lars Svensson <lars1.svensson@sonymobile.com>
---
 drivers/staging/fbtft/fbtft-bus.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/fbtft/fbtft-bus.c b/drivers/staging/fbtft/fbtft-bus.c
index f5a36a4..04efe90 100644
--- a/drivers/staging/fbtft/fbtft-bus.c
+++ b/drivers/staging/fbtft/fbtft-bus.c
@@ -184,7 +184,7 @@ EXPORT_SYMBOL(fbtft_write_vmem16_bus8);
 /* 16 bit pixel over 9-bit SPI bus: dc + high byte, dc + low byte */
 int fbtft_write_vmem16_bus9(struct fbtft_par *par, size_t offset, size_t len)
 {
-	u8 __iomem *vmem8;
+	u8 *vmem8;
 	u16 *txbuf16 = par->txbuf.buf;
 	size_t remain;
 	size_t to_copy;
@@ -212,12 +212,12 @@ int fbtft_write_vmem16_bus9(struct fbtft_par *par, size_t offset, size_t len)
 
 #ifdef __LITTLE_ENDIAN
 		for (i = 0; i < to_copy; i += 2) {
-			txbuf16[i]     = 0x0100 | ioread8(vmem8 + i + 1);
-			txbuf16[i + 1] = 0x0100 | ioread8(vmem8 + i);
+			txbuf16[i]     = 0x0100 | vmem8[i + 1];
+			txbuf16[i + 1] = 0x0100 | vmem8[i];
 		}
 #else
 		for (i = 0; i < to_copy; i++)
-			txbuf16[i]   = 0x0100 | ioread8(vmem8 + i);
+			txbuf16[i]   = 0x0100 | vmem8[i];
 #endif
 		vmem8 = vmem8 + to_copy;
 		ret = par->fbtftops.write(par, par->txbuf.buf, to_copy*2);
-- 
2.4.2


^ permalink raw reply related

* Re: [PATCH v3 1/3] fb.h: Provide alternate screen_base pointer
From: Dan Carpenter @ 2015-10-07  7:42 UTC (permalink / raw)
  To: Lars Svensson
  Cc: thomas.petazzoni, noralf, plagnioj, tomi.valkeinen, linux-fbdev,
	gregkh, devel, linux-kernel
In-Reply-To: <1444202414-21271-1-git-send-email-lars1.svensson@sonymobile.com>

Thanks!

I feel like this series is definitely the right thing, but I wonder how
we're going to merge it...  Anyway that's above my pay grade.

regards,
dan carpenter


^ permalink raw reply

* Re: [PATCH] video: of: fix memory leak
From: Tomi Valkeinen @ 2015-10-07 11:14 UTC (permalink / raw)
  To: Sudip Mukherjee, Jean-Christophe Plagniol-Villard
  Cc: linux-kernel, linux-fbdev
In-Reply-To: <1443606848-31510-1-git-send-email-sudipm.mukherjee@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 830 bytes --]


On 30/09/15 12:54, Sudip Mukherjee wrote:
> If of_parse_display_timing() fails we are printing an error message and
> jumping to the error path but we missed freeing "dt".
> 
> Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> ---
>  drivers/video/of_display_timing.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/video/of_display_timing.c b/drivers/video/of_display_timing.c
> index 32d8275..8a1076b 100644
> --- a/drivers/video/of_display_timing.c
> +++ b/drivers/video/of_display_timing.c
> @@ -210,6 +210,7 @@ struct display_timings *of_get_display_timings(struct device_node *np)
>  			 */
>  			pr_err("%s: error in timing %d\n",
>  				of_node_full_name(np), disp->num_timings + 1);
> +			kfree(dt);
>  			goto timingfail;
>  		}

Thanks, queued for 4.3 fixes.

 Tomi


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* [GIT PULL] fbdev fixes for 4.3
From: Tomi Valkeinen @ 2015-10-07 12:04 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linux-fbdev, linux-kernel@vger.kernel.org

[-- Attachment #1: Type: text/plain, Size: 1914 bytes --]

Hi Linus,

The following changes since commit 1f93e4a96c9109378204c147b3eec0d0e8100fde:

  Linux 4.3-rc2 (2015-09-20 14:32:34 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/tomba/linux.git tags/fbdev-fixes-4.3

for you to fetch changes up to d663baba8b16f3097ac3fb5baf97c54970d2f1ff:

  video: of: fix memory leak (2015-10-07 14:13:59 +0300)

----------------------------------------------------------------
fbdev fixes for v4.3

* fbdev: Minor fixes to broadsheetfb, fsl-diu-fb, mb862xxfb, tridentfb, omapfb
* display-timing: Fix memory leak in error path

----------------------------------------------------------------
Jason Jin (1):
      video: fbdev: fsl: Fix the sleep function for FSL DIU module

Javier Martinez Canillas (1):
      OMAPDSS: panel-sony-acx565akm: Export OF module alias information

Luis de Bethencourt (1):
      video: fbdev: mb862xx: Fix module autoload for OF platform driver

Ondrej Zary (2):
      tridentfb: fix hang on Blade3D with CONFIG_CC_OPTIMIZE_FOR_SIZE
      tridentfb: Fix set_lwidth on TGUI9440 and CYBER9320

Sudip Mukherjee (2):
      fbdev: broadsheetfb: fix memory leak
      video: of: fix memory leak

Vladimir Zapolskiy (1):
      fbdev: omap2: connector-dvi: use of_get_i2c_adapter_by_node interface

 drivers/video/fbdev/broadsheetfb.c                           |  8 +++++---
 drivers/video/fbdev/fsl-diu-fb.c                             |  9 ++++++++-
 drivers/video/fbdev/mb862xx/mb862xxfbdrv.c                   |  1 +
 drivers/video/fbdev/omap2/displays-new/connector-dvi.c       |  2 +-
 .../video/fbdev/omap2/displays-new/panel-sony-acx565akm.c    |  1 +
 drivers/video/fbdev/tridentfb.c                              | 12 +++++++++---
 drivers/video/of_display_timing.c                            |  1 +
 7 files changed, 26 insertions(+), 8 deletions(-)


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCH v2] video: fbdev: add Marvell PXA framebuffer binding
From: Philipp Zabel @ 2015-10-08  6:49 UTC (permalink / raw)
  To: Robert Jarzmik
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <87y4ffaj3m.fsf-4ty26DBLk+jEm7gnYqmdkQ@public.gmane.org>

On Tue, Oct 6, 2015 at 9:32 PM, Robert Jarzmik <robert.jarzmik@free.fr> wrote:
> Rob Herring <robh@kernel.org> writes:
>>> +       "marvell,pxa2xx-lcdc",
>>
>> No differences in h/w for any of the chips?
> All pxa25x, pxa27x and pxa3xx are compatible.
> AFAIK, pxa3xx has an IP with additional registers. But :
>  - these (this) register(s) is not necessary for the display controller to work
>    (it's more a control to shift red/green/blue values, and energy management)
>  - all the registers in pxa2{5,7}x are the same in pxa3xx

So the pxa3xx is pxa2xx compatible. The pxa3xx device tree should
contain compatible = "marvell,pxa3xx-lcdc", "marvell,pxa2xx-lcdc";

That way we can just ignore the pxa3xx compatible value until the need
arises to do something different with the additional registers, and if
that happens we already have it in the device trees.

>> If lcd-type is smart-panel, then this node would not make sense.
> Ah I see, so this could be optional maybe ?
>
> Actually when I'll move the panel definition to bindings/display/panel, is this
> what I should do :
>  - create a file marvell,pxa2xx-panel
>  - input all these properties into this file
>
> And then, when a board maintainer will create a devicetree description, he will
> write something like :
>       compatible = "toshiba,ltm0305a776";
>       compatible = "marvell,pxa2xx-panel";
>       lcd-type = "color-tft";
>       ...
>
> If that's the case, I wonder how to "enforce" that a panel used with
> marvell,pxa2xx-lcdc (through the of_graph 'port' node) be compatible with
> marvell,pxa2xx-panel ?

I think we should move to use drm simple-panel as soon as possible,
but until this information can be derived from the compatible string,
it should maybe stay contained to the lcdc node. At least I don't
think adding a marvell panel compatible is a solution. Maybe this
could be generalized and added as an optional property to the
simple-panel bindings.

regards
Philipp

^ permalink raw reply

* Re: [PATCH v2 0/5] [resend] gxt4500: Make usable on x86 and fix some bugs
From: Tomi Valkeinen @ 2015-10-08  9:21 UTC (permalink / raw)
  To: Ondrej Zary, Paul Mackerras; +Cc: linux-fbdev, Kernel development list
In-Reply-To: <1443734575-12468-1-git-send-email-linux@rainbow-software.org>

[-- Attachment #1: Type: text/plain, Size: 299 bytes --]



On 02/10/15 00:22, Ondrej Zary wrote:
> Fire GL2 AGP has GXT6000P and is a x86 card but the gxt4500 driver is
> currently limited to PPC.
> 
> This patch series makes the driver work on x86, fixes some color problems
> and enhances performance.
> 

Thanks, applying for 4.4.

 Tomi


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCH] radeonfb: Deinline large functions
From: Tomi Valkeinen @ 2015-10-08  9:26 UTC (permalink / raw)
  To: Denys Vlasenko, Benjamin Herrenschmidt
  Cc: Jean-Christophe Plagniol-Villard, Richard Purdie, Chris Wilson,
	David Airlie, Alex Deucher, Ben Skeggs, Zhang Rui, Len Brown,
	Jesse Barnes, Dave Airlie, linux-kernel, linux-fbdev
In-Reply-To: <1443031073-7039-1-git-send-email-dvlasenk@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 1127 bytes --]



On 23/09/15 20:57, Denys Vlasenko wrote:
> With this .config: http://busybox.net/~vda/kernel_config,
> after uninlining these functions have sizes and callsite counts
> as follows:
> 
> __OUTPLLP: 61 bytes, 12 callsites
> __INPLL:   79 bytes, 150 callsites
> __OUTPLL:  82 bytes, 138 callsites
> _OUTREGP: 101 bytes, 8 callsites
> _radeon_msleep:      66 bytes, 18 callsites
> _radeon_fifo_wait:   83 bytes, 24 callsites
> _radeon_engine_idle: 92 bytes, 10 callsites
> radeon_engine_flush: 105 bytes, 2 callsites
> radeon_pll_errata_after_index_slow: 31 bytes, 11 callsites
> radeon_pll_errata_after_data_slow:  91 bytes, 9 callsites
> 
> radeon_pll_errata_after_FOO functions are split into two parts:
> the inlined part which checks corresponding rinfo->errata bit,
> and out-of-line part which performs workaround magic per se.
> 
> Reduction in code size is about 49,500 bytes:
> 
>     text     data      bss       dec     hex filename
> 85789648 22294616 20627456 128711720 7abfc28 vmlinux.before
> 85740176 22294680 20627456 128662312 7ab3b28 vmlinux

Thanks, applying for 4.4.

 Tomi


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCH] uvesafb: make scaling configurable on Nvidia cards
From: Tomi Valkeinen @ 2015-10-08  9:31 UTC (permalink / raw)
  To: Mikulas Patocka, Michal Januszewski
  Cc: linux-fbdev, Jean-Christophe Plagniol-Villard, linux-kernel
In-Reply-To: <alpine.LRH.2.02.1509021719170.12073@file01.intranet.prod.int.rdu2.redhat.com>

[-- Attachment #1: Type: text/plain, Size: 1395 bytes --]


On 03/09/15 00:23, Mikulas Patocka wrote:
> [ I sent this some times ago, but didn't get any response ]
> 
> 
> Nvidia cards have a BIOS function 0x4f14 that allows to set flat panel
> scaling. This patch adds a module parameter "scaling" that uses this
> function to set the scaling. By default, the parameter is -1, so that the
> driver doesn't attempt to call the scaling function.
> 
> This patch is useful when using the binary Nvidia graphics driver - in
> that case, the console may be only in text mode or VESA mode. By default,
> the video card does scaling that degrades font quality and changes aspect
> ratio. This patch makes it possible to turn off the scaling and improve
> font quality on the console.
> 
> The allowed values depend on VESA BIOS. On my card, the following values
> are allowed:
> -1	- do not change the scaling
> 0	- scale to full screen
> 1, 2	- don't scale
> 3	- scale and preserve aspect ratio
> 4	- scale with black border around
> 
> Example use:
> echo 1 >/sys/module/uvesafb/parameters/scaling; fbset 1280x1024-60 -depth 32 -a
> - this sets unscaled 1280x1024 video mode that has much sharper font than
>   the scaled modes.

I have to say I don't know much about x86 video, but isn't uvesafb
supposed to be a generic driver? Adding nvidia specific features there
doesn't sound like a right way to handle this.

 Tomi


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCHv4 0/3] backlight: led-backlight driver
From: Tomi Valkeinen @ 2015-10-08  9:35 UTC (permalink / raw)
  To: Jacek Anaszewski, Jingoo Han, Lee Jones, linux-leds, linux-fbdev
  Cc: Andrew Lunn
In-Reply-To: <1443605522-1118-1-git-send-email-tomi.valkeinen@ti.com>

[-- Attachment #1: Type: text/plain, Size: 2257 bytes --]

Hi,

On 30/09/15 12:31, Tomi Valkeinen wrote:
> This series aims to add a led-backlight driver, similar to pwm-backlight, but
> using a LED class device underneath.
> 
> LED framework has no support for DT or getting a LED class driver from another
> kernel driver, so I added minimal functionality to led-class to get
> led-backlight working.
> 
> Changes to v3:
> - Change a comment to refer to of_led_get()
> 
> Changes to v2:
> - power supply is now optional
> - cosmetic changes
> - no-op function for led_put() when !CONFIG_LEDS_CLASS
> 
> Changes to v1:
> - Split LED OF parts into separate .h and .c files
> - Check for CONFIG_OF and CONFIG_LEDS_CLASS where relevant to leave unused code
>   out.
> - Improved error prints and comments a bit
> - Added put_device() into led_put(), as the device was gotten from
>   class_find_device() which requires a put_device() call.
> 
>  Tomi
> 
> Tomi Valkeinen (3):
>   leds: Add of_led_get() and led_put()
>   backlight: add led-backlight driver
>   devicetree: Add led-backlight binding
> 
>  .../bindings/video/backlight/led-backlight.txt     |  30 +++
>  drivers/leds/Makefile                              |   6 +-
>  drivers/leds/led-class.c                           |  13 +-
>  drivers/leds/led-of.c                              |  85 +++++++
>  drivers/leds/leds.h                                |   1 +
>  drivers/video/backlight/Kconfig                    |   7 +
>  drivers/video/backlight/Makefile                   |   1 +
>  drivers/video/backlight/led_bl.c                   | 246 +++++++++++++++++++++
>  include/linux/leds.h                               |  10 +
>  include/linux/of_leds.h                            |  26 +++
>  10 files changed, 423 insertions(+), 2 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/video/backlight/led-backlight.txt
>  create mode 100644 drivers/leds/led-of.c
>  create mode 100644 drivers/video/backlight/led_bl.c
>  create mode 100644 include/linux/of_leds.h

There's been no more comments on this. Should this be merged via led or
backlight trees?

Possible conflicts probably happen on the led side, as this changes the
led core files, so perhaps that's easier way?

 Tomi


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox