SUPERH platform development
 help / color / mirror / Atom feed
* Re: [PATCH] serial: sh-sci: don't filter on DMA device, use only
From: Vinod Koul @ 2011-08-30 10:14 UTC (permalink / raw)
  To: Guennadi Liakhovetski
  Cc: Magnus Damm, Paul Mundt, linux-sh, linux-kernel, Dan Williams,
	Magnus Damm
In-Reply-To: <Pine.LNX.4.64.1108300931220.19151@axis700.grange>

On Tue, 2011-08-30 at 09:54 +0200, Guennadi Liakhovetski wrote:
> On Mon, 29 Aug 2011, Vinod Koul wrote:
> > > >> You should not assign chan->private.
> > > >> Please move this  to dma_slave_control API
> > > >
> > > > I haven't seen any reply to this comment and this patch seems to still be
> > > > outstanding, is there an updated version of this patch that I've missed?
> > I don't recall seeing any updated version fixing this 
> 
> As a matter of fact, when you say "use dma_slave_control API," you 
> actually mean the dma_slave_config, right? Then, how is one supposed to 
> use it in this case? Shall we be issuing a DMA_SLAVE_CONFIG call inside of 
> the filter and check the return code? The problem is, that not all DMA 
> controllers on sh-mobile SoCs can service the same slave devices. So, if 
> we don't check in the filter we might well get an unsuitable DMA channel.
Here you are assigning to chan->private your specific values, which
should be moved to the dma_slave_config. 
But here you are removing the checks, and accepting the first channel
you got, so how do you find channel is suitable or not.

-- 
~Vinod


^ permalink raw reply

* Re: [PATCH] serial: sh-sci: don't filter on DMA device, use only
From: Guennadi Liakhovetski @ 2011-08-30 11:20 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Magnus Damm, Paul Mundt, linux-sh, linux-kernel, Dan Williams,
	Magnus Damm
In-Reply-To: <1314698522.1606.164.camel@vkoul-udesk3>

On Tue, 30 Aug 2011, Vinod Koul wrote:

> On Tue, 2011-08-30 at 09:54 +0200, Guennadi Liakhovetski wrote:
> > On Mon, 29 Aug 2011, Vinod Koul wrote:
> > > > >> You should not assign chan->private.
> > > > >> Please move this  to dma_slave_control API
> > > > >
> > > > > I haven't seen any reply to this comment and this patch seems to still be
> > > > > outstanding, is there an updated version of this patch that I've missed?
> > > I don't recall seeing any updated version fixing this 
> > 
> > As a matter of fact, when you say "use dma_slave_control API," you 
> > actually mean the dma_slave_config, right? Then, how is one supposed to 
> > use it in this case? Shall we be issuing a DMA_SLAVE_CONFIG call inside of 
> > the filter and check the return code? The problem is, that not all DMA 
> > controllers on sh-mobile SoCs can service the same slave devices. So, if 
> > we don't check in the filter we might well get an unsuitable DMA channel.
> Here you are assigning to chan->private your specific values, which
> should be moved to the dma_slave_config. 
> But here you are removing the checks, and accepting the first channel
> you got, so how do you find channel is suitable or not.

That's done in the driver's .device_alloc_chan_resources() method. It 
checkx the .private pointer, tries to find a suitable channel, if it fails 
- it returns -EINVAL. See 
drivers/dma/shdma.c::sh_dmae_alloc_chan_resources():

	if (param) {
		const struct sh_dmae_slave_config *cfg;

		cfg = sh_dmae_find_slave(sh_chan, param);
		if (!cfg) {
			ret = -EINVAL;
			goto efindslave;
		}

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

^ permalink raw reply

* Re: [PATCH] dma: shdma: transfer based runtime PM
From: Guennadi Liakhovetski @ 2011-08-30 11:32 UTC (permalink / raw)
  To: Vinod Koul; +Cc: linux-kernel, linux-sh, Dan Williams, Paul Mundt
In-Reply-To: <1314698257.1606.159.camel@vkoul-udesk3>

On Tue, 30 Aug 2011, Vinod Koul wrote:

> On Tue, 2011-08-30 at 09:12 +0200, Guennadi Liakhovetski wrote:
> > On Mon, 29 Aug 2011, Vinod Koul wrote:
> > > > > Something like:
> > > > > /* since callback is set for last descriptor of chain, we call runtime
> > > > >  * put for that desc alone
> > > > >  */
> > > > > list_for_each_entry_safe(desc, __desc, sh_chan->ld_queue, node) {
> > > > > 	if (desc->async_tx.callback)
> > > > > 		pm_runtime_put(device);
> > > > 
> > > > Not all dma users have callbacks.
> > > Do you have such usage today, at least I dont :)
> > > Nevertheless, in tx_submit adding a simple flag in your drivers
> > > descriptor structure can tell you whether to call _put() or not. Agreed?
> > 
> > Yes, I agree, that one could make this work too. Still, I do not 
> > understand how and why this is better to the extent, that I have to 
> > reimplement my patch, retest and resubmit it. Maybe Dan or Paul have an 
> > opinion on this?
> But wont it make code look simpler and cleaner, you don't reply on your
> counters but on pm_runtime infrastructure to do the job.

Sorry, I see it differently. I don't use any counters in my patch. I'm 
only checking for empty queue, i.e., I'm just identifying the first 
descriptor submission and the last completion or termination.

> You juts need
> to call _put/_get at right places, which IMO l;ooks lot simpler than
> current approach

If we didn't have to check for exact symmetry, then yes, I agree, this 
would be cleaner. I.e., if we indeed had well-defined entry- and 
exit-points, which are guaranteed to be called exact same number of times. 
Like, e.g., with file open() / close() etc. But since we don't have this 
symmetry, and instead have to add flags and iterate lists, this doesn't 
look natural and simple to me anymore, sorry.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

^ permalink raw reply

* Re: [PATCH] serial: sh-sci: don't filter on DMA device, use only
From: Vinod Koul @ 2011-08-30 11:35 UTC (permalink / raw)
  To: Guennadi Liakhovetski
  Cc: Magnus Damm, Paul Mundt, linux-sh, linux-kernel, Dan Williams,
	Magnus Damm
In-Reply-To: <Pine.LNX.4.64.1108301317280.19151@axis700.grange>

On Tue, 2011-08-30 at 13:20 +0200, Guennadi Liakhovetski wrote:
> On Tue, 30 Aug 2011, Vinod Koul wrote:
> 
> > On Tue, 2011-08-30 at 09:54 +0200, Guennadi Liakhovetski wrote:
> > > On Mon, 29 Aug 2011, Vinod Koul wrote:
> > > > > >> You should not assign chan->private.
> > > > > >> Please move this  to dma_slave_control API
> > > > > >
> > > > > > I haven't seen any reply to this comment and this patch seems to still be
> > > > > > outstanding, is there an updated version of this patch that I've missed?
> > > > I don't recall seeing any updated version fixing this 
> > > 
> > > As a matter of fact, when you say "use dma_slave_control API," you 
> > > actually mean the dma_slave_config, right? Then, how is one supposed to 
> > > use it in this case? Shall we be issuing a DMA_SLAVE_CONFIG call inside of 
> > > the filter and check the return code? The problem is, that not all DMA 
> > > controllers on sh-mobile SoCs can service the same slave devices. So, if 
> > > we don't check in the filter we might well get an unsuitable DMA channel.
> > Here you are assigning to chan->private your specific values, which
> > should be moved to the dma_slave_config. 
> > But here you are removing the checks, and accepting the first channel
> > you got, so how do you find channel is suitable or not.
> 
> That's done in the driver's .device_alloc_chan_resources() method. It 
> checkx the .private pointer, tries to find a suitable channel, if it fails 
> - it returns -EINVAL. See 
> drivers/dma/shdma.c::sh_dmae_alloc_chan_resources():
> 
> 	if (param) {
> 		const struct sh_dmae_slave_config *cfg;
> 
> 		cfg = sh_dmae_find_slave(sh_chan, param);
> 		if (!cfg) {
> 			ret = -EINVAL;
> 			goto efindslave;
> 		}
Now am doubly confused. Are you saying that you are using
alloc_chan_resources for doing filtering??

-- 
~Vinod


^ permalink raw reply

* Re: [PATCH] serial: sh-sci: don't filter on DMA device, use only
From: Guennadi Liakhovetski @ 2011-08-30 11:40 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Magnus Damm, Paul Mundt, linux-sh, linux-kernel, Dan Williams,
	Magnus Damm
In-Reply-To: <1314703426.1606.167.camel@vkoul-udesk3>

On Tue, 30 Aug 2011, Vinod Koul wrote:

> On Tue, 2011-08-30 at 13:20 +0200, Guennadi Liakhovetski wrote:
> > On Tue, 30 Aug 2011, Vinod Koul wrote:
> > 
> > > On Tue, 2011-08-30 at 09:54 +0200, Guennadi Liakhovetski wrote:
> > > > On Mon, 29 Aug 2011, Vinod Koul wrote:
> > > > > > >> You should not assign chan->private.
> > > > > > >> Please move this  to dma_slave_control API
> > > > > > >
> > > > > > > I haven't seen any reply to this comment and this patch seems to still be
> > > > > > > outstanding, is there an updated version of this patch that I've missed?
> > > > > I don't recall seeing any updated version fixing this 
> > > > 
> > > > As a matter of fact, when you say "use dma_slave_control API," you 
> > > > actually mean the dma_slave_config, right? Then, how is one supposed to 
> > > > use it in this case? Shall we be issuing a DMA_SLAVE_CONFIG call inside of 
> > > > the filter and check the return code? The problem is, that not all DMA 
> > > > controllers on sh-mobile SoCs can service the same slave devices. So, if 
> > > > we don't check in the filter we might well get an unsuitable DMA channel.
> > > Here you are assigning to chan->private your specific values, which
> > > should be moved to the dma_slave_config. 
> > > But here you are removing the checks, and accepting the first channel
> > > you got, so how do you find channel is suitable or not.
> > 
> > That's done in the driver's .device_alloc_chan_resources() method. It 
> > checkx the .private pointer, tries to find a suitable channel, if it fails 
> > - it returns -EINVAL. See 
> > drivers/dma/shdma.c::sh_dmae_alloc_chan_resources():
> > 
> > 	if (param) {
> > 		const struct sh_dmae_slave_config *cfg;
> > 
> > 		cfg = sh_dmae_find_slave(sh_chan, param);
> > 		if (!cfg) {
> > 			ret = -EINVAL;
> > 			goto efindslave;
> > 		}
> Now am doubly confused. Are you saying that you are using
> alloc_chan_resources for doing filtering??

Let's look at __dma_request_channel(). It iterates over DMA devices and 
calls private_candidate() for each of them, which in turn calls the filter 
function for each free channel on the current device. As soon as the 
filter returns "true" for one of channels, a private candidate is found. 
And in my filter I do exactly this - assign the .private pointer and 
return "true." Next dma_chan_get() is called, which in turn calls driver's 
.device_alloc_chan_resources() method. There we check the previously set 
.private pointer to see, if this slave can be served by this DMA device. 
On sh-mobile you can freely configure single DMA channels for different 
slaves, as long as this slave is at all supported by the current DMA 
controller device. If this slave can be supported - all is good, we use 
the private data to configure the channel. If not - we return an error and 
__dma_request_channel() iterates to the next DMA controller device, which 
is exactly what we need.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

^ permalink raw reply

* [PATCH] ARM: mach-shmobile: ag5evm needs CONFIG_I2C
From: Guennadi Liakhovetski @ 2011-08-30 16:19 UTC (permalink / raw)
  To: linux-sh

ag5evm implements a backlight control, using an I2C controller, therefore
it needs CONFIG_I2C to fix this make failure

arch/arm/mach-shmobile/built-in.o: In function `lcd_on':
pfc-sh73a0.c:(.text+0x2334): undefined reference to `i2c_get_adapter'
pfc-sh73a0.c:(.text+0x2370): undefined reference to `i2c_transfer'

(ignore pfc-sh73a0.c) and to build successfully.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 arch/arm/mach-shmobile/Kconfig |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-shmobile/Kconfig b/arch/arm/mach-shmobile/Kconfig
index 0c8f6cf..f68856c 100644
--- a/arch/arm/mach-shmobile/Kconfig
+++ b/arch/arm/mach-shmobile/Kconfig
@@ -26,6 +26,7 @@ config ARCH_SH73A0
 	select SH_CLK_CPG
 	select ARCH_WANT_OPTIONAL_GPIOLIB
 	select ARM_GIC
+	select I2C
 
 comment "SH-Mobile Board Type"
 
-- 
1.7.2.5


^ permalink raw reply related

* [PATCH 0/2] mmc: sh_mmcif: simplify platform DMA configuration
From: Guennadi Liakhovetski @ 2011-08-30 16:26 UTC (permalink / raw)
  To: linux-sh

A simple cosmetic clean-up, no functional changes. Patch 2/2 depends on 
patch 1/2 and can wait until 3.3. Paul, would you be able to put it under 
the carpet somewhere until then or shall I resend it after 3.2-rc1 is out? 
After both these patches have been applied, we can remove struct 
sh_mmcif_plat_data::dma around 3.4 or 4.0 or whatever;-)

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

^ permalink raw reply

* [PATCH 1/2] mmc: sh_mmcif: simplify platform data
From: Guennadi Liakhovetski @ 2011-08-30 16:26 UTC (permalink / raw)
  To: linux-mmc; +Cc: linux-sh, Paul Mundt, Chris Ball
In-Reply-To: <Pine.LNX.4.64.1108301819150.19151@axis700.grange>

Provide platforms with a simplified way to specify MMCIF DMA slave IDs in
a way, similar to SDHI and other sh_dma clients.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/mmc/host/sh_mmcif.c  |   20 ++++++++++++++++----
 include/linux/mmc/sh_mmcif.h |    4 +++-
 2 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
index 557886b..bd91c94 100644
--- a/drivers/mmc/host/sh_mmcif.c
+++ b/drivers/mmc/host/sh_mmcif.c
@@ -165,6 +165,8 @@ struct sh_mmcif_host {
 	struct mmc_host *mmc;
 	struct mmc_data *data;
 	struct platform_device *pd;
+	struct sh_dmae_slave dma_slave_tx;
+	struct sh_dmae_slave dma_slave_rx;
 	struct clk *hclk;
 	unsigned int clk;
 	int bus_width;
@@ -323,25 +325,35 @@ static bool sh_mmcif_filter(struct dma_chan *chan, void *arg)
 static void sh_mmcif_request_dma(struct sh_mmcif_host *host,
 				 struct sh_mmcif_plat_data *pdata)
 {
+	struct sh_dmae_slave *tx, *rx;
 	host->dma_active = false;
 
 	/* We can only either use DMA for both Tx and Rx or not use it at all */
 	if (pdata->dma) {
+		dev_warn(&host->pd->dev,
+			 "Update your platform to use embedded DMA slave IDs\n");
+		tx = &pdata->dma->chan_priv_tx;
+		rx = &pdata->dma->chan_priv_rx;
+	} else {
+		tx = &host->dma_slave_tx;
+		tx->slave_id = pdata->slave_id_tx;
+		rx = &host->dma_slave_rx;
+		rx->slave_id = pdata->slave_id_rx;
+	}
+	if (tx->slave_id > 0 && rx->slave_id > 0) {
 		dma_cap_mask_t mask;
 
 		dma_cap_zero(mask);
 		dma_cap_set(DMA_SLAVE, mask);
 
-		host->chan_tx = dma_request_channel(mask, sh_mmcif_filter,
-						    &pdata->dma->chan_priv_tx);
+		host->chan_tx = dma_request_channel(mask, sh_mmcif_filter, tx);
 		dev_dbg(&host->pd->dev, "%s: TX: got channel %p\n", __func__,
 			host->chan_tx);
 
 		if (!host->chan_tx)
 			return;
 
-		host->chan_rx = dma_request_channel(mask, sh_mmcif_filter,
-						    &pdata->dma->chan_priv_rx);
+		host->chan_rx = dma_request_channel(mask, sh_mmcif_filter, rx);
 		dev_dbg(&host->pd->dev, "%s: RX: got channel %p\n", __func__,
 			host->chan_rx);
 
diff --git a/include/linux/mmc/sh_mmcif.h b/include/linux/mmc/sh_mmcif.h
index 0222cd8..04ff452 100644
--- a/include/linux/mmc/sh_mmcif.h
+++ b/include/linux/mmc/sh_mmcif.h
@@ -41,7 +41,9 @@ struct sh_mmcif_plat_data {
 	void (*set_pwr)(struct platform_device *pdev, int state);
 	void (*down_pwr)(struct platform_device *pdev);
 	int (*get_cd)(struct platform_device *pdef);
-	struct sh_mmcif_dma	*dma;
+	struct sh_mmcif_dma	*dma;		/* Deprecated. Instead */
+	unsigned int		slave_id_tx;	/* use embedded slave_id_[tr]x */
+	unsigned int		slave_id_rx;
 	u8			sup_pclk;	/* 1 :SH7757, 0: SH7724/SH7372 */
 	unsigned long		caps;
 	u32			ocr;
-- 
1.7.2.5


^ permalink raw reply related

* [PATCH 2/2] ARM: mach-shmobile: simplify MMCIF DMA configuration
From: Guennadi Liakhovetski @ 2011-08-30 16:26 UTC (permalink / raw)
  To: linux-mmc; +Cc: linux-sh, Paul Mundt, Chris Ball
In-Reply-To: <Pine.LNX.4.64.1108301819150.19151@axis700.grange>

Use the simplified method to specify MMCIF DMA slave configuration.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 arch/arm/mach-shmobile/board-ag5evm.c   |   11 ++---------
 arch/arm/mach-shmobile/board-ap4evb.c   |   12 ++----------
 arch/arm/mach-shmobile/board-mackerel.c |   12 ++----------
 3 files changed, 6 insertions(+), 29 deletions(-)

diff --git a/arch/arm/mach-shmobile/board-ag5evm.c b/arch/arm/mach-shmobile/board-ag5evm.c
index ce5c251..7f26ecc 100644
--- a/arch/arm/mach-shmobile/board-ag5evm.c
+++ b/arch/arm/mach-shmobile/board-ag5evm.c
@@ -158,19 +158,12 @@ static struct resource sh_mmcif_resources[] = {
 	},
 };
 
-static struct sh_mmcif_dma sh_mmcif_dma = {
-	.chan_priv_rx	= {
-		.slave_id	= SHDMA_SLAVE_MMCIF_RX,
-	},
-	.chan_priv_tx	= {
-		.slave_id	= SHDMA_SLAVE_MMCIF_TX,
-	},
-};
 static struct sh_mmcif_plat_data sh_mmcif_platdata = {
 	.sup_pclk	= 0,
 	.ocr		= MMC_VDD_165_195,
 	.caps		= MMC_CAP_8_BIT_DATA | MMC_CAP_NONREMOVABLE,
-	.dma		= &sh_mmcif_dma,
+	.slave_id_tx	= SHDMA_SLAVE_MMCIF_TX,
+	.slave_id_rx	= SHDMA_SLAVE_MMCIF_RX,
 };
 
 static struct platform_device mmc_device = {
diff --git a/arch/arm/mach-shmobile/board-ap4evb.c b/arch/arm/mach-shmobile/board-ap4evb.c
index f530bf2..b8ed7a7 100644
--- a/arch/arm/mach-shmobile/board-ap4evb.c
+++ b/arch/arm/mach-shmobile/board-ap4evb.c
@@ -293,15 +293,6 @@ static struct resource sh_mmcif_resources[] = {
 	},
 };
 
-static struct sh_mmcif_dma sh_mmcif_dma = {
-	.chan_priv_rx	= {
-		.slave_id	= SHDMA_SLAVE_MMCIF_RX,
-	},
-	.chan_priv_tx	= {
-		.slave_id	= SHDMA_SLAVE_MMCIF_TX,
-	},
-};
-
 static struct sh_mmcif_plat_data sh_mmcif_plat = {
 	.sup_pclk	= 0,
 	.ocr		= MMC_VDD_165_195 | MMC_VDD_32_33 | MMC_VDD_33_34,
@@ -309,7 +300,8 @@ static struct sh_mmcif_plat_data sh_mmcif_plat = {
 			  MMC_CAP_8_BIT_DATA |
 			  MMC_CAP_NEEDS_POLL,
 	.get_cd		= slot_cn7_get_cd,
-	.dma		= &sh_mmcif_dma,
+	.slave_id_tx	= SHDMA_SLAVE_MMCIF_TX,
+	.slave_id_rx	= SHDMA_SLAVE_MMCIF_RX,
 };
 
 static struct platform_device sh_mmcif_device = {
diff --git a/arch/arm/mach-shmobile/board-mackerel.c b/arch/arm/mach-shmobile/board-mackerel.c
index 7e7da60..dfae824 100644
--- a/arch/arm/mach-shmobile/board-mackerel.c
+++ b/arch/arm/mach-shmobile/board-mackerel.c
@@ -1164,15 +1164,6 @@ static struct resource sh_mmcif_resources[] = {
 	},
 };
 
-static struct sh_mmcif_dma sh_mmcif_dma = {
-	.chan_priv_rx	= {
-		.slave_id	= SHDMA_SLAVE_MMCIF_RX,
-	},
-	.chan_priv_tx	= {
-		.slave_id	= SHDMA_SLAVE_MMCIF_TX,
-	},
-};
-
 static struct sh_mmcif_plat_data sh_mmcif_plat = {
 	.sup_pclk	= 0,
 	.ocr		= MMC_VDD_165_195 | MMC_VDD_32_33 | MMC_VDD_33_34,
@@ -1180,7 +1171,8 @@ static struct sh_mmcif_plat_data sh_mmcif_plat = {
 			  MMC_CAP_8_BIT_DATA |
 			  MMC_CAP_NEEDS_POLL,
 	.get_cd		= slot_cn7_get_cd,
-	.dma		= &sh_mmcif_dma,
+	.slave_id_tx	= SHDMA_SLAVE_MMCIF_TX,
+	.slave_id_rx	= SHDMA_SLAVE_MMCIF_RX,
 };
 
 static struct platform_device sh_mmcif_device = {
-- 
1.7.2.5


^ permalink raw reply related

* Re: [PULL] SH mobile LCDC cleanups and fixes
From: Florian Tobias Schandinat @ 2011-08-30 20:41 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <201108191044.52130.laurent.pinchart@ideasonboard.com>

Hi Laurent,

On 08/19/2011 08:44 AM, Laurent Pinchart wrote:
> Hi Florian,
> 
> Here are the latest SH mobile LCDC and MERAM cleanups and fixes based on top
> of v3.1-rc2. All the patches have been posted to the list. I've incorporated
> acked-by and tested-by lines when available, but most of the patches received
> no comment.
> 
> The following changes since commit 93ee7a9340d64f20295aacc3fb6a22b759323280:
> 
>   Linux 3.1-rc2 (2011-08-14 15:09:08 -0700)
> 
> are available in the git repository at:
>   git://linuxtv.org/pinchartl/fbdev.git sh-mobile-lcdc

Pulled, thanks.
It looks good to me, as far as I can tell.
Just the first commits that were not your own ones should have had more verbose
commit messages and I think they should have had your Signed-off as you received
them via e-mail?

And on another note:
Would you mind adding yourself as maintainer of drivers/video/sh_mobile_* in
MAINTAINERS? I can't find any entry for it yet and having me as primary source
for review for a driver I don't know is a bad idea, at least if there are people
that know it better.


Thanks,

Florian Tobias Schandinat

> 
> Damian Hobson-Garcia (5):
>       fbdev: sh_mobile_meram: Enable runtime PM
>       fbdev: sh_mobile_meram: Enable/disable MERAM along with LCDC
>       fbdev: sh_mobile_meram: Move private data from .h to .c
>       fbdev: sh_mobile_meram: Backup/restore device registers on shutdown/resume
>       fbdev: sh_mobile_meram: Assign meram to the SH7372_A4LC power domain
> 
> Laurent Pinchart (11):
>       fbdev: sh_mobile_lcdc: Turn dot clock on before resuming from runtime PM
>       fbdev: sh_mobile_lcdc: Replace hardcoded register values with macros
>       fbdev: sh_mobile_lcdc: Don't acknowlege interrupts unintentionally
>       fbdev: sh_mobile_lcdc: Compute clock pattern using divider denominator
>       fbdev: sh_mobile_lcdc: Split LCDC start code from sh_mobile_lcdc_start
>       fbdev: sh_mobile_lcdc: Store the frame buffer base address when panning
>       fbdev: sh_mobile_lcdc: Restart LCDC in runtime PM resume handler
>       fbdev: sh_mobile_meram: Replace hardcoded register values with macros
>       fbdev: sh_mobile_meram: Validate ICB configuration outside mutex
>       fbdev: sh_mobile_meram: Fix MExxCTL register save on runtime PM suspend
>       fbdev: sh_mobile_meram: Remove unneeded sh_mobile_meram.h
> 
>  arch/arm/mach-shmobile/board-mackerel.c |    1 +
>  drivers/video/sh_mobile_lcdcfb.c        |  592 ++++++++++++++-----------------
>  drivers/video/sh_mobile_lcdcfb.h        |   12 +-
>  drivers/video/sh_mobile_meram.c         |  202 +++++++++--
>  drivers/video/sh_mobile_meram.h         |   41 ---
>  include/video/sh_mobile_lcdc.h          |  135 ++++++-
>  6 files changed, 556 insertions(+), 427 deletions(-)
>  delete mode 100644 drivers/video/sh_mobile_meram.h
> 


^ permalink raw reply

* [PATCH 0/5] PM: Generic PM domains and device PM QoS
From: Rafael J. Wysocki @ 2011-08-30 22:17 UTC (permalink / raw)
  To: Linux PM mailing list
  Cc: LKML, Linux-sh list, Magnus Damm, Kevin Hilman, jean.pihet

Hi,

This patchset illustrates how device PM QoS may be used along with
PM domains in my view.

Actually, it consists of two parts.  Namely, patches [1-3/5] seem to be
suitable for 3.2, unless somebody hates them, but patches [4-5/5] are
total RFC.  They haven't been tested, only compiled, so the use of them
is not encouraged (they may kill your dog or make your cat go wild, or
do something equally nasty, so beware).  Their purpose is to illustrate
an idea that I'd like to discuss at the PM miniconference during the
LPC.

[1/5] - Split device PM domain data into a "base" and additional fields
        (one need_restore field at the moment, but the subsequent patches
        add more fields).

[2/5] - Make runtime PM always release power.lock if power.irq_safe is set.

[3/5] - Add function for reading device PM QoS values safely.

[4/5] - Add governor function for stopping devices.

[5/5] - Add generic PM domain power off governor function.

Thanks,
Rafael


^ permalink raw reply

* [PATCH 1/5] PM / Domains: Split device PM domain data into base and need_restore
From: Rafael J. Wysocki @ 2011-08-30 22:18 UTC (permalink / raw)
  To: Linux PM mailing list
  Cc: LKML, Linux-sh list, Magnus Damm, Kevin Hilman, jean.pihet
In-Reply-To: <201108310017.03103.rjw@sisk.pl>

From: Rafael J. Wysocki <rjw@sisk.pl>

The struct pm_domain_data data type is defined in such a way that
adding new fields specific to the generic PM domains code will
require include/linux/pm.h to be modified.  As a result, data types
used only by the generic PM domains code will be defined in two
headers, although they all should be defined in pm_domain.h and
pm.h will need to include more headers, which won't be very nice.

For this reason change the definition of struct pm_subsys_data
so that its domain_data member is a pointer, which will allow
struct pm_domain_data to be subclassed by various PM domains
implementations.  Remove the need_restore member from
struct pm_domain_data and make the generic PM domains code
subclass it by adding the need_restore member to the new data type.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/base/power/domain.c |   28 +++++++++++++++++++---------
 include/linux/pm.h          |    3 +--
 include/linux/pm_domain.h   |   10 ++++++++++
 3 files changed, 30 insertions(+), 11 deletions(-)

Index: linux/include/linux/pm.h
=================================--- linux.orig/include/linux/pm.h
+++ linux/include/linux/pm.h
@@ -433,7 +433,6 @@ struct wakeup_source;
 struct pm_domain_data {
 	struct list_head list_node;
 	struct device *dev;
-	bool need_restore;
 };
 
 struct pm_subsys_data {
@@ -443,7 +442,7 @@ struct pm_subsys_data {
 	struct list_head clock_list;
 #endif
 #ifdef CONFIG_PM_GENERIC_DOMAINS
-	struct pm_domain_data domain_data;
+	struct pm_domain_data *domain_data;
 #endif
 };
 
Index: linux/include/linux/pm_domain.h
=================================--- linux.orig/include/linux/pm_domain.h
+++ linux/include/linux/pm_domain.h
@@ -62,6 +62,16 @@ struct gpd_link {
 	struct list_head slave_node;
 };
 
+struct generic_pm_domain_data {
+	struct pm_domain_data base;
+	bool need_restore;
+};
+
+static inline struct generic_pm_domain_data *to_gpd_data(struct pm_domain_data *pdd)
+{
+	return container_of(pdd, struct generic_pm_domain_data, base);
+}
+
 #ifdef CONFIG_PM_GENERIC_DOMAINS
 extern int pm_genpd_add_device(struct generic_pm_domain *genpd,
 			       struct device *dev);
Index: linux/drivers/base/power/domain.c
=================================--- linux.orig/drivers/base/power/domain.c
+++ linux/drivers/base/power/domain.c
@@ -188,11 +188,12 @@ static int __pm_genpd_save_device(struct
 				  struct generic_pm_domain *genpd)
 	__releases(&genpd->lock) __acquires(&genpd->lock)
 {
+	struct generic_pm_domain_data *gpd_data = to_gpd_data(pdd);
 	struct device *dev = pdd->dev;
 	struct device_driver *drv = dev->driver;
 	int ret = 0;
 
-	if (pdd->need_restore)
+	if (gpd_data->need_restore)
 		return 0;
 
 	mutex_unlock(&genpd->lock);
@@ -210,7 +211,7 @@ static int __pm_genpd_save_device(struct
 	mutex_lock(&genpd->lock);
 
 	if (!ret)
-		pdd->need_restore = true;
+		gpd_data->need_restore = true;
 
 	return ret;
 }
@@ -224,10 +225,11 @@ static void __pm_genpd_restore_device(st
 				      struct generic_pm_domain *genpd)
 	__releases(&genpd->lock) __acquires(&genpd->lock)
 {
+	struct generic_pm_domain_data *gpd_data = to_gpd_data(pdd);
 	struct device *dev = pdd->dev;
 	struct device_driver *drv = dev->driver;
 
-	if (!pdd->need_restore)
+	if (!gpd_data->need_restore)
 		return;
 
 	mutex_unlock(&genpd->lock);
@@ -244,7 +246,7 @@ static void __pm_genpd_restore_device(st
 
 	mutex_lock(&genpd->lock);
 
-	pdd->need_restore = false;
+	gpd_data->need_restore = false;
 }
 
 /**
@@ -493,7 +495,7 @@ static int pm_genpd_runtime_resume(struc
 		mutex_lock(&genpd->lock);
 	}
 	finish_wait(&genpd->status_wait_queue, &wait);
-	__pm_genpd_restore_device(&dev->power.subsys_data->domain_data, genpd);
+	__pm_genpd_restore_device(dev->power.subsys_data->domain_data, genpd);
 	genpd->resume_count--;
 	genpd_set_active(genpd);
 	wake_up_all(&genpd->status_wait_queue);
@@ -1080,6 +1082,7 @@ static void pm_genpd_complete(struct dev
  */
 int pm_genpd_add_device(struct generic_pm_domain *genpd, struct device *dev)
 {
+	struct generic_pm_domain_data *gpd_data;
 	struct pm_domain_data *pdd;
 	int ret = 0;
 
@@ -1106,14 +1109,20 @@ int pm_genpd_add_device(struct generic_p
 			goto out;
 		}
 
+	gpd_data = kzalloc(sizeof(*gpd_data), GFP_KERNEL);
+	if (!gpd_data) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
 	genpd->device_count++;
 
 	dev->pm_domain = &genpd->domain;
 	dev_pm_get_subsys_data(dev);
-	pdd = &dev->power.subsys_data->domain_data;
-	pdd->dev = dev;
-	pdd->need_restore = false;
-	list_add_tail(&pdd->list_node, &genpd->dev_list);
+	dev->power.subsys_data->domain_data = &gpd_data->base;
+	gpd_data->base.dev = dev;
+	gpd_data->need_restore = false;
+	list_add_tail(&gpd_data->base.list_node, &genpd->dev_list);
 
  out:
 	genpd_release_lock(genpd);
@@ -1152,6 +1161,7 @@ int pm_genpd_remove_device(struct generi
 		pdd->dev = NULL;
 		dev_pm_put_subsys_data(dev);
 		dev->pm_domain = NULL;
+		kfree(to_gpd_data(pdd));
 
 		genpd->device_count--;
 


^ permalink raw reply

* [PATCH 2/5] PM / Runtime: Do not run callbacks under lock for power.irq_safe set
From: Rafael J. Wysocki @ 2011-08-30 22:20 UTC (permalink / raw)
  To: Linux PM mailing list
  Cc: LKML, Linux-sh list, Magnus Damm, Kevin Hilman, jean.pihet
In-Reply-To: <201108310017.03103.rjw@sisk.pl>

From: Rafael J. Wysocki <rjw@sisk.pl>

The rpm_suspend() and rpm_resume() routines execute subsystem or PM
domain callbacks under power.lock if power.irq_safe is set for the
given device.  This is inconsistent with that rpm_idle() does after
commit 02b2677 (PM / Runtime: Allow _put_sync() from
interrupts-disabled context) and is problematic for subsystems and PM
domains wanting to use power.lock for synchronization in their
runtime PM callbacks.  For this reason, make runtime PM core functions
always release power.lock before invoking subsystem or PM domain
callbacks.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/base/power/runtime.c |   50 ++++++++++++++++++++++++-------------------
 1 file changed, 28 insertions(+), 22 deletions(-)

Index: linux/drivers/base/power/runtime.c
=================================--- linux.orig/drivers/base/power/runtime.c
+++ linux/drivers/base/power/runtime.c
@@ -155,6 +155,31 @@ static int rpm_check_suspend_allowed(str
 }
 
 /**
+ * __rpm_callback - Run a given runtime PM callback for a given device.
+ * @cb: Runtime PM callback to run.
+ * @dev: Device to run the callback for.
+ */
+static int __rpm_callback(int (*cb)(struct device *), struct device *dev)
+	__releases(&dev->power.lock) __acquires(&dev->power.lock)
+{
+	int retval;
+
+	if (dev->power.irq_safe)
+		spin_unlock(&dev->power.lock);
+	else
+		spin_unlock_irq(&dev->power.lock);
+
+	retval = cb(dev);
+
+	if (dev->power.irq_safe)
+		spin_lock(&dev->power.lock);
+	else
+		spin_lock_irq(&dev->power.lock);
+
+	return retval;
+}
+
+/**
  * rpm_idle - Notify device bus type if the device can be suspended.
  * @dev: Device to notify the bus type about.
  * @rpmflags: Flag bits.
@@ -225,19 +250,8 @@ static int rpm_idle(struct device *dev,
 	else
 		callback = NULL;
 
-	if (callback) {
-		if (dev->power.irq_safe)
-			spin_unlock(&dev->power.lock);
-		else
-			spin_unlock_irq(&dev->power.lock);
-
-		callback(dev);
-
-		if (dev->power.irq_safe)
-			spin_lock(&dev->power.lock);
-		else
-			spin_lock_irq(&dev->power.lock);
-	}
+	if (callback)
+		__rpm_callback(callback, dev);
 
 	dev->power.idle_notification = false;
 	wake_up_all(&dev->power.wait_queue);
@@ -252,22 +266,14 @@ static int rpm_idle(struct device *dev,
  * @dev: Device to run the callback for.
  */
 static int rpm_callback(int (*cb)(struct device *), struct device *dev)
-	__releases(&dev->power.lock) __acquires(&dev->power.lock)
 {
 	int retval;
 
 	if (!cb)
 		return -ENOSYS;
 
-	if (dev->power.irq_safe) {
-		retval = cb(dev);
-	} else {
-		spin_unlock_irq(&dev->power.lock);
-
-		retval = cb(dev);
+	retval = __rpm_callback(cb, dev);
 
-		spin_lock_irq(&dev->power.lock);
-	}
 	dev->power.runtime_error = retval;
 	return retval != -EACCES ? retval : -EIO;
 }


^ permalink raw reply

* [PATCH 3/5] PM / QoS: Add function dev_pm_qos_read_value()
From: Rafael J. Wysocki @ 2011-08-30 22:21 UTC (permalink / raw)
  To: Linux PM mailing list
  Cc: LKML, Linux-sh list, Magnus Damm, Kevin Hilman, jean.pihet
In-Reply-To: <201108310017.03103.rjw@sisk.pl>

From: Rafael J. Wysocki <rjw@sisk.pl>

To read the current PM QoS value for a given device we need to
make sure that the device's power.constraints object won't be
removed while we're doing that.  For this reason, put the
operation under dev->power.lock and acquire the lock
around the initialization and removal of power.constraints.

Moreover, since we're using the value of power.constraints to
determine whether or not the object is present, the
power.constraints_state field isn't necessary any more and may be
removed.  However, dev_pm_qos_add_request() needs to check if the
device is being removed from the system before allocating a new
PM QoS constraints object for it, so it has to use device_pm_lock()
and the device PM QoS initialization and destruction should be done
under device_pm_lock() as well.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/base/power/main.c |    4 -
 drivers/base/power/qos.c  |  167 ++++++++++++++++++++++++++--------------------
 include/linux/pm.h        |    8 --
 include/linux/pm_qos.h    |    3 
 4 files changed, 101 insertions(+), 81 deletions(-)

Index: linux/drivers/base/power/qos.c
=================================--- linux.orig/drivers/base/power/qos.c
+++ linux/drivers/base/power/qos.c
@@ -30,15 +30,6 @@
  * . To minimize the data usage by the per-device constraints, the data struct
  *   is only allocated at the first call to dev_pm_qos_add_request.
  * . The data is later free'd when the device is removed from the system.
- * . The constraints_state variable from dev_pm_info tracks the data struct
- *    allocation state:
- *    DEV_PM_QOS_NO_DEVICE: No device present or device removed, no data
- *     allocated,
- *    DEV_PM_QOS_DEVICE_PRESENT: Device present, data not allocated and will be
- *     allocated at the first call to dev_pm_qos_add_request,
- *    DEV_PM_QOS_ALLOCATED: Device present, data allocated. The per-device
- *     PM QoS constraints framework is operational and constraints can be
- *     added, updated or removed using the dev_pm_qos_* API.
  *  . A global mutex protects the constraints users from the data being
  *     allocated and free'd.
  */
@@ -51,8 +42,30 @@
 
 
 static DEFINE_MUTEX(dev_pm_qos_mtx);
+
 static BLOCKING_NOTIFIER_HEAD(dev_pm_notifiers);
 
+/**
+ * dev_pm_qos_read_value - Get PM QoS constraint for a given device.
+ * @dev: Device to get the PM QoS constraint value for.
+ */
+s32 dev_pm_qos_read_value(struct device *dev)
+{
+	struct pm_qos_constraints *c;
+	unsigned long flags;
+	s32 ret = 0;
+
+	spin_lock_irqsave(&dev->power.lock, flags);
+
+	c = dev->power.constraints;
+	if (c)
+		ret = pm_qos_read_value(c);
+
+	spin_unlock_irqrestore(&dev->power.lock, flags);
+
+	return ret;
+}
+
 /*
  * apply_constraint
  * @req: constraint request to apply
@@ -105,27 +118,37 @@ static int dev_pm_qos_constraints_alloca
 	}
 	BLOCKING_INIT_NOTIFIER_HEAD(n);
 
+	plist_head_init(&c->list);
+	c->target_value = PM_QOS_DEV_LAT_DEFAULT_VALUE;
+	c->default_value = PM_QOS_DEV_LAT_DEFAULT_VALUE;
+	c->type = PM_QOS_MIN;
+	c->notifiers = n;
+
+	spin_lock_irq(&dev->power.lock);
 	dev->power.constraints = c;
-	plist_head_init(&dev->power.constraints->list);
-	dev->power.constraints->target_value = PM_QOS_DEV_LAT_DEFAULT_VALUE;
-	dev->power.constraints->default_value =	PM_QOS_DEV_LAT_DEFAULT_VALUE;
-	dev->power.constraints->type = PM_QOS_MIN;
-	dev->power.constraints->notifiers = n;
-	dev->power.constraints_state = DEV_PM_QOS_ALLOCATED;
+	spin_unlock_irq(&dev->power.lock);
 
 	return 0;
 }
 
+static void __dev_pm_qos_constraints_init(struct device *dev)
+{
+	spin_lock_irq(&dev->power.lock);
+	dev->power.constraints = NULL;
+	spin_unlock_irq(&dev->power.lock);
+}
+
 /**
- * dev_pm_qos_constraints_init
+ * dev_pm_qos_constraints_init - Initalize device's PM QoS constraints pointer.
  * @dev: target device
  *
- * Called from the device PM subsystem at device insertion
+ * Called from the device PM subsystem at device insertion under
+ * device_pm_lock().
  */
 void dev_pm_qos_constraints_init(struct device *dev)
 {
 	mutex_lock(&dev_pm_qos_mtx);
-	dev->power.constraints_state = DEV_PM_QOS_DEVICE_PRESENT;
+	dev->power.constraints = NULL;
 	mutex_unlock(&dev_pm_qos_mtx);
 }
 
@@ -133,34 +156,35 @@ void dev_pm_qos_constraints_init(struct
  * dev_pm_qos_constraints_destroy
  * @dev: target device
  *
- * Called from the device PM subsystem at device removal
+ * Called from the device PM subsystem at device removal under device_pm_lock().
  */
 void dev_pm_qos_constraints_destroy(struct device *dev)
 {
 	struct dev_pm_qos_request *req, *tmp;
+	struct pm_qos_constraints *c;
 
 	mutex_lock(&dev_pm_qos_mtx);
 
-	if (dev->power.constraints_state = DEV_PM_QOS_ALLOCATED) {
-		/* Flush the constraints list for the device */
-		plist_for_each_entry_safe(req, tmp,
-					  &dev->power.constraints->list,
-					  node) {
-			/*
-			 * Update constraints list and call the notification
-			 * callbacks if needed
-			 */
-			apply_constraint(req, PM_QOS_REMOVE_REQ,
-					 PM_QOS_DEFAULT_VALUE);
-			memset(req, 0, sizeof(*req));
-		}
+	c = dev->power.constraints;
+	if (!c)
+		goto out;
 
-		kfree(dev->power.constraints->notifiers);
-		kfree(dev->power.constraints);
-		dev->power.constraints = NULL;
+	/* Flush the constraints list for the device */
+	plist_for_each_entry_safe(req, tmp, &c->list, node) {
+		/*
+		 * Update constraints list and call the notification
+		 * callbacks if needed
+		 */
+		apply_constraint(req, PM_QOS_REMOVE_REQ, PM_QOS_DEFAULT_VALUE);
+		memset(req, 0, sizeof(*req));
 	}
-	dev->power.constraints_state = DEV_PM_QOS_NO_DEVICE;
 
+	__dev_pm_qos_constraints_init(dev);
+
+	kfree(c->notifiers);
+	kfree(c);
+
+ out:
 	mutex_unlock(&dev_pm_qos_mtx);
 }
 
@@ -178,8 +202,8 @@ void dev_pm_qos_constraints_destroy(stru
  *
  * Returns 1 if the aggregated constraint value has changed,
  * 0 if the aggregated constraint value has not changed,
- * -EINVAL in case of wrong parameters, -ENODEV if the device has been
- * removed from the system
+ * -EINVAL in case of wrong parameters, -ENOMEM if there's not enough memory
+ * to allocate for data structures.
  */
 int dev_pm_qos_add_request(struct device *dev, struct dev_pm_qos_request *req,
 			   s32 value)
@@ -195,28 +219,35 @@ int dev_pm_qos_add_request(struct device
 		return -EINVAL;
 	}
 
-	mutex_lock(&dev_pm_qos_mtx);
 	req->dev = dev;
 
-	/* Return if the device has been removed */
-	if (req->dev->power.constraints_state = DEV_PM_QOS_NO_DEVICE) {
-		ret = -ENODEV;
-		goto out;
-	}
+	device_pm_lock();
+	mutex_lock(&dev_pm_qos_mtx);
 
-	/*
-	 * Allocate the constraints data on the first call to add_request,
-	 * i.e. only if the data is not already allocated and if the device has
-	 * not been removed
-	 */
-	if (dev->power.constraints_state = DEV_PM_QOS_DEVICE_PRESENT)
-		ret = dev_pm_qos_constraints_allocate(dev);
+	if (dev->power.constraints) {
+		device_pm_unlock();
+	} else {
+		if (list_empty(&dev->power.entry)) {
+			/* The device has been removed from the system. */
+			device_pm_unlock();
+			goto out;
+		} else {
+			device_pm_unlock();
+			/*
+			 * Allocate the constraints data on the first call to
+			 * add_request, i.e. only if the data is not already
+			 * allocated and if the device has not been removed.
+			 */
+			ret = dev_pm_qos_constraints_allocate(dev);
+		}
+	}
 
 	if (!ret)
 		ret = apply_constraint(req, PM_QOS_ADD_REQ, value);
 
-out:
+ out:
 	mutex_unlock(&dev_pm_qos_mtx);
+
 	return ret;
 }
 EXPORT_SYMBOL_GPL(dev_pm_qos_add_request);
@@ -252,13 +283,13 @@ int dev_pm_qos_update_request(struct dev
 
 	mutex_lock(&dev_pm_qos_mtx);
 
-	if (req->dev->power.constraints_state = DEV_PM_QOS_ALLOCATED) {
+	if (req->dev->power.constraints) {
 		if (new_value != req->node.prio)
 			ret = apply_constraint(req, PM_QOS_UPDATE_REQ,
 					       new_value);
 	} else {
 		/* Return if the device has been removed */
-		ret = -ENODEV;
+		ret = -EINVAL;
 	}
 
 	mutex_unlock(&dev_pm_qos_mtx);
@@ -293,7 +324,7 @@ int dev_pm_qos_remove_request(struct dev
 
 	mutex_lock(&dev_pm_qos_mtx);
 
-	if (req->dev->power.constraints_state = DEV_PM_QOS_ALLOCATED) {
+	if (req->dev->power.constraints) {
 		ret = apply_constraint(req, PM_QOS_REMOVE_REQ,
 				       PM_QOS_DEFAULT_VALUE);
 		memset(req, 0, sizeof(*req));
@@ -323,15 +354,12 @@ int dev_pm_qos_add_notifier(struct devic
 
 	mutex_lock(&dev_pm_qos_mtx);
 
-	/* Silently return if the device has been removed */
-	if (dev->power.constraints_state != DEV_PM_QOS_ALLOCATED)
-		goto out;
-
-	retval = blocking_notifier_chain_register(
-			dev->power.constraints->notifiers,
-			notifier);
+	/* Silently return if the constraints object is not present. */
+	if (dev->power.constraints)
+		retval = blocking_notifier_chain_register(
+				dev->power.constraints->notifiers,
+				notifier);
 
-out:
 	mutex_unlock(&dev_pm_qos_mtx);
 	return retval;
 }
@@ -354,15 +382,12 @@ int dev_pm_qos_remove_notifier(struct de
 
 	mutex_lock(&dev_pm_qos_mtx);
 
-	/* Silently return if the device has been removed */
-	if (dev->power.constraints_state != DEV_PM_QOS_ALLOCATED)
-		goto out;
-
-	retval = blocking_notifier_chain_unregister(
-			dev->power.constraints->notifiers,
-			notifier);
+	/* Silently return if the constraints object is not present. */
+	if (dev->power.constraints)
+		retval = blocking_notifier_chain_unregister(
+				dev->power.constraints->notifiers,
+				notifier);
 
-out:
 	mutex_unlock(&dev_pm_qos_mtx);
 	return retval;
 }
Index: linux/include/linux/pm_qos.h
=================================--- linux.orig/include/linux/pm_qos.h
+++ linux/include/linux/pm_qos.h
@@ -77,6 +77,7 @@ int pm_qos_remove_notifier(int pm_qos_cl
 int pm_qos_request_active(struct pm_qos_request *req);
 s32 pm_qos_read_value(struct pm_qos_constraints *c);
 
+s32 dev_pm_qos_read_value(struct device *dev);
 int dev_pm_qos_add_request(struct device *dev, struct dev_pm_qos_request *req,
 			   s32 value);
 int dev_pm_qos_update_request(struct dev_pm_qos_request *req, s32 new_value);
@@ -117,6 +118,8 @@ static inline int pm_qos_request_active(
 static inline s32 pm_qos_read_value(struct pm_qos_constraints *c)
 			{ return 0; }
 
+static inline s32 dev_pm_qos_read_value(struct device *dev)
+			{ return 0; }
 static inline int dev_pm_qos_add_request(struct device *dev,
 					 struct dev_pm_qos_request *req,
 					 s32 value)
Index: linux/drivers/base/power/main.c
=================================--- linux.orig/drivers/base/power/main.c
+++ linux/drivers/base/power/main.c
@@ -98,8 +98,8 @@ void device_pm_add(struct device *dev)
 		dev_warn(dev, "parent %s should not be sleeping\n",
 			dev_name(dev->parent));
 	list_add_tail(&dev->power.entry, &dpm_list);
-	mutex_unlock(&dpm_list_mtx);
 	dev_pm_qos_constraints_init(dev);
+	mutex_unlock(&dpm_list_mtx);
 }
 
 /**
@@ -110,9 +110,9 @@ void device_pm_remove(struct device *dev
 {
 	pr_debug("PM: Removing info for %s:%s\n",
 		 dev->bus ? dev->bus->name : "No Bus", dev_name(dev));
-	dev_pm_qos_constraints_destroy(dev);
 	complete_all(&dev->power.completion);
 	mutex_lock(&dpm_list_mtx);
+	dev_pm_qos_constraints_destroy(dev);
 	list_del_init(&dev->power.entry);
 	mutex_unlock(&dpm_list_mtx);
 	device_wakeup_disable(dev);
Index: linux/include/linux/pm.h
=================================--- linux.orig/include/linux/pm.h
+++ linux/include/linux/pm.h
@@ -421,13 +421,6 @@ enum rpm_request {
 	RPM_REQ_RESUME,
 };
 
-/* Per-device PM QoS constraints data struct state */
-enum dev_pm_qos_state {
-	DEV_PM_QOS_NO_DEVICE,		/* No device present */
-	DEV_PM_QOS_DEVICE_PRESENT,	/* Device present, data not allocated */
-	DEV_PM_QOS_ALLOCATED,		/* Device present, data allocated */
-};
-
 struct wakeup_source;
 
 struct pm_domain_data {
@@ -489,7 +482,6 @@ struct dev_pm_info {
 #endif
 	struct pm_subsys_data	*subsys_data;  /* Owned by the subsystem. */
 	struct pm_qos_constraints *constraints;
-	enum dev_pm_qos_state	constraints_state;
 };
 
 extern void update_pm_runtime_accounting(struct device *dev);


^ permalink raw reply

* [RFC][PATCH 4/5] PM / Domains: Add device stop governor function
From: Rafael J. Wysocki @ 2011-08-30 22:21 UTC (permalink / raw)
  To: Linux PM mailing list
  Cc: LKML, Linux-sh list, Magnus Damm, Kevin Hilman, jean.pihet
In-Reply-To: <201108310017.03103.rjw@sisk.pl>

From: Rafael J. Wysocki <rjw@sisk.pl>

Add a function deciding whether or not devices should be
stopped in pm_genpd_runtime_suspend() depending on their
PM QoS values.

---
 arch/arm/mach-shmobile/pm-sh7372.c   |    2 -
 drivers/base/power/Makefile          |    2 -
 drivers/base/power/domain.c          |   35 ++++++++++++++++++-----
 drivers/base/power/domain_governor.c |   40 ++++++++++++++++++++++++++
 include/linux/pm_domain.h            |   52 ++++++++++++++++++++++++++++++-----
 5 files changed, 115 insertions(+), 16 deletions(-)

Index: linux/include/linux/pm_domain.h
=================================--- linux.orig/include/linux/pm_domain.h
+++ linux/include/linux/pm_domain.h
@@ -21,6 +21,7 @@ enum gpd_status {
 
 struct dev_power_governor {
 	bool (*power_down_ok)(struct dev_pm_domain *domain);
+	bool (*stop_ok)(struct device *dev);
 };
 
 struct generic_pm_domain {
@@ -62,8 +63,13 @@ struct gpd_link {
 	struct list_head slave_node;
 };
 
+struct gpd_gov_dev_data {
+	s64 break_even_ns;
+};
+
 struct generic_pm_domain_data {
 	struct pm_domain_data base;
+	struct gpd_gov_dev_data *gov_data;
 	bool need_restore;
 };
 
@@ -73,18 +79,47 @@ static inline struct generic_pm_domain_d
 }
 
 #ifdef CONFIG_PM_GENERIC_DOMAINS
-extern int pm_genpd_add_device(struct generic_pm_domain *genpd,
-			       struct device *dev);
+extern struct dev_power_governor default_qos_governor;
+
+extern struct generic_pm_domain *dev_to_genpd(struct device *dev);
+extern int __pm_genpd_add_device(struct generic_pm_domain *genpd,
+				 struct device *dev,
+				 struct gpd_gov_dev_data *gov_data);
+
+static inline int pm_genpd_add_device(struct generic_pm_domain *genpd,
+				      struct device *dev)
+{
+	return __pm_genpd_add_device(genpd, dev, NULL);
+}
+
 extern int pm_genpd_remove_device(struct generic_pm_domain *genpd,
 				  struct device *dev);
 extern int pm_genpd_add_subdomain(struct generic_pm_domain *genpd,
 				  struct generic_pm_domain *new_subdomain);
 extern int pm_genpd_remove_subdomain(struct generic_pm_domain *genpd,
 				     struct generic_pm_domain *target);
-extern void pm_genpd_init(struct generic_pm_domain *genpd,
-			  struct dev_power_governor *gov, bool is_off);
+extern void __pm_genpd_init(struct generic_pm_domain *genpd,
+			    struct dev_power_governor *gov, bool is_off);
+
+static inline void pm_genpd_init(struct generic_pm_domain *genpd, bool is_off)
+{
+	__pm_genpd_init(genpd, &default_qos_governor, is_off);
+}
+
 extern int pm_genpd_poweron(struct generic_pm_domain *genpd);
+
 #else
+
+static inline struct generic_pm_domain *dev_to_genpd(struct device *dev)
+{
+	return ERR_PTR(-ENOSYS);
+}
+static inline int __pm_genpd_add_device(struct generic_pm_domain *genpd,
+					struct device *dev,
+					struct gpd_gov_dev_data *gov_data)
+{
+	return -ENOSYS;
+}
 static inline int pm_genpd_add_device(struct generic_pm_domain *genpd,
 				      struct device *dev)
 {
@@ -105,8 +140,13 @@ static inline int pm_genpd_remove_subdom
 {
 	return -ENOSYS;
 }
-static inline void pm_genpd_init(struct generic_pm_domain *genpd,
-				 struct dev_power_governor *gov, bool is_off) {}
+static inline void __pm_genpd_init(struct generic_pm_domain *genpd,
+				   struct dev_power_governor *gov, bool is_off)
+{
+}
+static inline void pm_genpd_init(struct generic_pm_domain *genpd, bool is_off)
+{
+}
 static inline int pm_genpd_poweron(struct generic_pm_domain *genpd)
 {
 	return -ENOSYS;
Index: linux/drivers/base/power/domain.c
=================================--- linux.orig/drivers/base/power/domain.c
+++ linux/drivers/base/power/domain.c
@@ -21,7 +21,7 @@ static DEFINE_MUTEX(gpd_list_lock);
 
 #ifdef CONFIG_PM
 
-static struct generic_pm_domain *dev_to_genpd(struct device *dev)
+struct generic_pm_domain *dev_to_genpd(struct device *dev)
 {
 	if (IS_ERR_OR_NULL(dev->pm_domain))
 		return ERR_PTR(-EINVAL);
@@ -403,6 +403,22 @@ static void genpd_power_off_work_fn(stru
 }
 
 /**
+ * genpd_stop_dev - Stop a given device if that's beneficial.
+ * @genpd: PM domain the device belongs to.
+ * @dev: Device to stop.
+ */
+static int genpd_stop_dev(struct generic_pm_domain *genpd, struct device *dev)
+{
+	bool (*stop_ok)(struct device *dev);
+
+	stop_ok = genpd->gov ? genpd->gov->stop_ok : NULL;
+	if (stop_ok && !stop_ok(dev))
+		return -EBUSY;
+
+	return genpd->stop_device(dev);
+}
+
+/**
  * pm_genpd_runtime_suspend - Suspend a device belonging to I/O PM domain.
  * @dev: Device to suspend.
  *
@@ -423,7 +439,7 @@ static int pm_genpd_runtime_suspend(stru
 	might_sleep_if(!genpd->dev_irq_safe);
 
 	if (genpd->stop_device) {
-		int ret = genpd->stop_device(dev);
+		int ret = genpd_stop_dev(genpd, dev);
 		if (ret)
 			return ret;
 	}
@@ -495,7 +511,7 @@ static int pm_genpd_runtime_resume(struc
 		mutex_lock(&genpd->lock);
 	}
 	finish_wait(&genpd->status_wait_queue, &wait);
-	__pm_genpd_restore_device(dev->power.subsys_data->domain_data, genpd);
+	__pm_genpd_restore_device(dev_to_psd(dev)->domain_data, genpd);
 	genpd->resume_count--;
 	genpd_set_active(genpd);
 	wake_up_all(&genpd->status_wait_queue);
@@ -1076,11 +1092,13 @@ static void pm_genpd_complete(struct dev
 #endif /* CONFIG_PM_SLEEP */
 
 /**
- * pm_genpd_add_device - Add a device to an I/O PM domain.
+ * __pm_genpd_add_device - Add a device to an I/O PM domain.
  * @genpd: PM domain to add the device to.
  * @dev: Device to be added.
+ * @gov_data: Set of PM QoS parameters to attach to the device.
  */
-int pm_genpd_add_device(struct generic_pm_domain *genpd, struct device *dev)
+int __pm_genpd_add_device(struct generic_pm_domain *genpd, struct device *dev,
+			  struct gpd_gov_dev_data *gov_data)
 {
 	struct generic_pm_domain_data *gpd_data;
 	struct pm_domain_data *pdd;
@@ -1123,6 +1141,7 @@ int pm_genpd_add_device(struct generic_p
 	gpd_data->base.dev = dev;
 	gpd_data->need_restore = false;
 	list_add_tail(&gpd_data->base.list_node, &genpd->dev_list);
+	gpd_data->gov_data = gov_data;
 
  out:
 	genpd_release_lock(genpd);
@@ -1280,13 +1299,13 @@ int pm_genpd_remove_subdomain(struct gen
 }
 
 /**
- * pm_genpd_init - Initialize a generic I/O PM domain object.
+ * __pm_genpd_init - Initialize a generic I/O PM domain object.
  * @genpd: PM domain object to initialize.
  * @gov: PM domain governor to associate with the domain (may be NULL).
  * @is_off: Initial value of the domain's power_is_off field.
  */
-void pm_genpd_init(struct generic_pm_domain *genpd,
-		   struct dev_power_governor *gov, bool is_off)
+void __pm_genpd_init(struct generic_pm_domain *genpd,
+		     struct dev_power_governor *gov, bool is_off)
 {
 	if (IS_ERR_OR_NULL(genpd))
 		return;
Index: linux/drivers/base/power/Makefile
=================================--- linux.orig/drivers/base/power/Makefile
+++ linux/drivers/base/power/Makefile
@@ -3,7 +3,7 @@ obj-$(CONFIG_PM_SLEEP)	+= main.o wakeup.
 obj-$(CONFIG_PM_RUNTIME)	+= runtime.o
 obj-$(CONFIG_PM_TRACE_RTC)	+= trace.o
 obj-$(CONFIG_PM_OPP)	+= opp.o
-obj-$(CONFIG_PM_GENERIC_DOMAINS)	+=  domain.o
+obj-$(CONFIG_PM_GENERIC_DOMAINS)	+=  domain.o domain_governor.o
 obj-$(CONFIG_HAVE_CLK)	+= clock_ops.o
 
 ccflags-$(CONFIG_DEBUG_DRIVER) := -DDEBUG
Index: linux/drivers/base/power/domain_governor.c
=================================--- /dev/null
+++ linux/drivers/base/power/domain_governor.c
@@ -0,0 +1,40 @@
+/*
+ * drivers/base/power/domain_governor.c - Governors for device PM domains.
+ *
+ * Copyright (C) 2011 Rafael J. Wysocki <rjw@sisk.pl>, Renesas Electronics Corp.
+ *
+ * This file is released under the GPLv2.
+ */
+
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/pm_domain.h>
+#include <linux/pm_qos.h>
+
+static bool default_stop_ok(struct device *dev)
+{
+	struct gpd_gov_dev_data *gov_data;
+	s64 constraint_ns;
+	s32 constraint;
+
+	dev_dbg(dev, "%s()\n", __func__);
+
+	gov_data = to_gpd_data(dev_to_psd(dev)->domain_data)->gov_data;
+	if (!gov_data)
+		return true;
+
+	constraint = dev_pm_qos_read_value(dev);
+	if (constraint < 0)
+		return false;
+	else if (constraint = 0) /* 0 means "don't care" */
+		return true;
+
+	constraint_ns = constraint;
+	constraint_ns *= NSEC_PER_USEC;
+
+	return constraint_ns > gov_data->break_even_ns;
+}
+
+struct dev_power_governor default_qos_governor = {
+	.stop_ok = default_stop_ok,
+};
Index: linux/arch/arm/mach-shmobile/pm-sh7372.c
=================================--- linux.orig/arch/arm/mach-shmobile/pm-sh7372.c
+++ linux/arch/arm/mach-shmobile/pm-sh7372.c
@@ -100,7 +100,7 @@ void sh7372_init_pm_domain(struct sh7372
 {
 	struct generic_pm_domain *genpd = &sh7372_pd->genpd;
 
-	pm_genpd_init(genpd, NULL, false);
+	pm_genpd_init(genpd, false);
 	genpd->stop_device = pm_clk_suspend;
 	genpd->start_device = pm_clk_resume;
 	genpd->dev_irq_safe = true;


^ permalink raw reply

* [RFC][PATCH 5/5] PM / Domains: Add default power off governor function
From: Rafael J. Wysocki @ 2011-08-30 22:22 UTC (permalink / raw)
  To: Linux PM mailing list
  Cc: LKML, Linux-sh list, Magnus Damm, Kevin Hilman, jean.pihet
In-Reply-To: <201108310017.03103.rjw@sisk.pl>

From: Rafael J. Wysocki <rjw@sisk.pl>

Add a function deciding whether or not a given PM domain should
be powered off on the basis of that domain's devices' PM QoS
constraints.

---
 drivers/base/power/domain_governor.c |   96 +++++++++++++++++++++++++++++++++++
 include/linux/pm_domain.h            |    7 ++
 2 files changed, 103 insertions(+)

Index: linux/include/linux/pm_domain.h
=================================--- linux.orig/include/linux/pm_domain.h
+++ linux/include/linux/pm_domain.h
@@ -49,6 +49,10 @@ struct generic_pm_domain {
 	int (*start_device)(struct device *dev);
 	int (*stop_device)(struct device *dev);
 	bool (*active_wakeup)(struct device *dev);
+	ktime_t power_off_latency;
+	ktime_t power_on_latency;
+	s64 break_even_ns;
+	s64 min_delta_ns;
 };
 
 static inline struct generic_pm_domain *pd_to_genpd(struct dev_pm_domain *pd)
@@ -64,6 +68,9 @@ struct gpd_link {
 };
 
 struct gpd_gov_dev_data {
+	ktime_t start_latency;
+	ktime_t suspend_latency;
+	ktime_t resume_latency;
 	s64 break_even_ns;
 };
 
Index: linux/drivers/base/power/domain_governor.c
=================================--- linux.orig/drivers/base/power/domain_governor.c
+++ linux/drivers/base/power/domain_governor.c
@@ -35,6 +35,102 @@ static bool default_stop_ok(struct devic
 	return constraint_ns > gov_data->break_even_ns;
 }
 
+/* This routine must be executed under the PM domain's lock. */
+static bool default_power_down_ok(struct dev_pm_domain *pd)
+{
+	struct generic_pm_domain *genpd = pd_to_genpd(pd);
+	struct gpd_link *link;
+	struct pm_domain_data *pdd;
+	ktime_t off_time, on_time;
+	s64 delta_ns, min_delta_ns;
+
+	on_time = genpd->power_on_latency;
+	/* Check if slave domains can be off for enough time. */
+	delta_ns = ktime_to_ns(ktime_add(genpd->power_off_latency, on_time));
+	min_delta_ns = 0;
+	/* All slave domains have been powered off at this point. */
+	list_for_each_entry(link, &genpd->master_links, master_node) {
+		if (delta_ns > link->slave->min_delta_ns)
+			return false;
+
+		delta_ns = link->slave->min_delta_ns - delta_ns;
+		if (delta_ns < min_delta_ns)
+			min_delta_ns = delta_ns;
+	}
+
+	genpd->min_delta_ns = min_delta_ns;
+
+	/* Compute the total time needed to power off the domain. */
+	off_time = ktime_set(0, 0);
+	/* All devices have been stopped at this point. */
+	list_for_each_entry(pdd, &genpd->dev_list, list_node) {
+		struct gpd_gov_dev_data *gov_data;
+
+		if (!pdd->dev->driver)
+			continue;
+
+		gov_data = to_gpd_data(pdd)->gov_data;
+		if (!gov_data)
+			continue;
+
+		off_time = ktime_add(off_time, gov_data->suspend_latency);
+	}
+	off_time = ktime_add(off_time, genpd->power_off_latency);
+
+	/*
+	 * For each device in the domain compute the difference between the
+	 * QoS value and the total time required to bring the device back
+	 * assuming that the domain will be powered off and compute the minimum
+	 * of those.
+	 */
+	min_delta_ns = 0;
+	on_time = ktime_add(on_time, off_time);
+	list_for_each_entry(pdd, &genpd->dev_list, list_node) {
+		struct gpd_gov_dev_data *gov_data;
+		struct device *dev = pdd->dev;
+		ktime_t dev_up_time;
+		s32 constraint;
+		s64 constraint_ns;
+
+		if (!dev->driver)
+			continue;
+
+		gov_data = to_gpd_data(pdd)->gov_data;
+		if (gov_data) {
+			dev_up_time = ktime_add(on_time,
+						gov_data->resume_latency);
+			dev_up_time = ktime_add(dev_up_time,
+						gov_data->start_latency);
+		} else {
+			dev_up_time = on_time;
+		}
+
+		constraint = dev_pm_qos_read_value(dev);
+		if (constraint < 0)
+			return false;
+		else if (constraint = 0) /* 0 means "don't care" */
+			continue;
+
+		constraint_ns = constraint;
+		constraint_ns *= NSEC_PER_USEC;
+		delta_ns = constraint_ns - ktime_to_ns(dev_up_time);
+		if (min_delta_ns > delta_ns)
+			min_delta_ns = delta_ns;
+	}
+
+	/* Compare the computed delta with the break even value. */
+	if (min_delta_ns < genpd->break_even_ns)
+		return false;
+
+	/* Store the computed value for the masters to use. */
+	if (genpd->min_delta_ns > min_delta_ns)
+		genpd->min_delta_ns = min_delta_ns;
+
+	/* The domain can be powered off. */
+	return true;
+}
+
 struct dev_power_governor default_qos_governor = {
 	.stop_ok = default_stop_ok,
+	.power_down_ok = default_power_down_ok,
 };


^ permalink raw reply

* [PATCH v2 0/8] sh_mobile_lcdc: Support format changes at runtime
From: Laurent Pinchart @ 2011-08-31 11:00 UTC (permalink / raw)
  To: linux-fbdev

Hi everybody,

Here's the second version of the .fb_set_par() support for the sh_mobile_lcdc
driver patch set. It allows userspace to change the frame buffer format and
size at runtime. The patches apply on top of Florian's fbdev-next branch
available at git://github.com/schandinat/linux-2.6.git.

Compared to v1, this version includes an additional MERAM fix.

Frame buffer memory reallocation is currently not supported. You will need to
make sure platform data sets the maximum size and bpp to high enough values
for all formats and sizes you want to support at runtime.

The patches have been tested with the fbdev-test utility
(http://git.ideasonboard.org/?pûdev-test.git;a=summary) on a Mackerel board.

Laurent Pinchart (8):
  sh_mobile_meram: Reset ICBs at unregistration time
  fbdev: sh_mobile_lcdc: Adjust requested parameters in .fb_check_var
  fbdev: sh_mobile_lcdc: Add support for format changes at runtime
  fbdev: sh_mobile_lcdc: use display information in info for panning
  fbdev: sh_mobile_lcdc: Update fix.line_length in .fb_set_par()
  fbdev: sh_mobile_lcdc: Avoid forward declarations
  fbdev: sh_mobile_lcdc: Split channel initialization from probe
    function
  fbdev: sh_mobile_lcdc: Remove sh_mobile_lcdc_set_bpp()

 drivers/video/sh_mobile_lcdcfb.c |  570 +++++++++++++++++++++-----------------
 drivers/video/sh_mobile_meram.c  |    6 +-
 2 files changed, 313 insertions(+), 263 deletions(-)

-- 
Regards,

Laurent Pinchart


^ permalink raw reply

* [PATCH v2 1/8] sh_mobile_meram: Reset ICBs at unregistration time
From: Laurent Pinchart @ 2011-08-31 11:00 UTC (permalink / raw)
  To: linux-fbdev

When ICBs are unregistered and later reused they need to be reset to
avoid data corruption. Set the WBF, WF and RF bits to make sure ICBs get
reset properly.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/video/sh_mobile_meram.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/video/sh_mobile_meram.c b/drivers/video/sh_mobile_meram.c
index f632970..4d63490 100644
--- a/drivers/video/sh_mobile_meram.c
+++ b/drivers/video/sh_mobile_meram.c
@@ -373,8 +373,10 @@ static void meram_deinit(struct sh_mobile_meram_priv *priv,
 			struct sh_mobile_meram_icb *icb)
 {
 	/* disable ICB */
-	meram_write_icb(priv->base, icb->cache_icb,  MExxCTL, 0);
-	meram_write_icb(priv->base, icb->marker_icb, MExxCTL, 0);
+	meram_write_icb(priv->base, icb->cache_icb,  MExxCTL,
+			MExxCTL_WBF | MExxCTL_WF | MExxCTL_RF);
+	meram_write_icb(priv->base, icb->marker_icb, MExxCTL,
+			MExxCTL_WBF | MExxCTL_WF | MExxCTL_RF);
 	icb->cache_unit = 0;
 }
 
-- 
1.7.3.4


^ permalink raw reply related

* [PATCH v2 2/8] fbdev: sh_mobile_lcdc: Adjust requested parameters in .fb_check_var
From: Laurent Pinchart @ 2011-08-31 11:00 UTC (permalink / raw)
  To: linux-fbdev

Instead of failing when the requested fb_var_screeninfo parameters are
not supported, adjust the parameters according to the hardware
capabilities.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/video/sh_mobile_lcdcfb.c |  103 ++++++++++++++++++++++++++++++++------
 1 files changed, 88 insertions(+), 15 deletions(-)

diff --git a/drivers/video/sh_mobile_lcdcfb.c b/drivers/video/sh_mobile_lcdcfb.c
index 088cb17..33b0ff8 100644
--- a/drivers/video/sh_mobile_lcdcfb.c
+++ b/drivers/video/sh_mobile_lcdcfb.c
@@ -1055,28 +1055,101 @@ static int sh_mobile_check_var(struct fb_var_screeninfo *var, struct fb_info *in
 {
 	struct sh_mobile_lcdc_chan *ch = info->par;
 	struct sh_mobile_lcdc_priv *p = ch->lcdc;
+	unsigned int best_dist = (unsigned int)-1;
+	unsigned int best_xres = 0;
+	unsigned int best_yres = 0;
+	unsigned int i;
 
-	if (var->xres > MAX_XRES || var->yres > MAX_YRES ||
-	    var->xres * var->yres * (ch->cfg.bpp / 8) * 2 > info->fix.smem_len) {
-		dev_warn(info->dev, "Invalid info: %u-%u-%u-%u x %u-%u-%u-%u @ %lukHz!\n",
-			 var->left_margin, var->xres, var->right_margin, var->hsync_len,
-			 var->upper_margin, var->yres, var->lower_margin, var->vsync_len,
-			 PICOS2KHZ(var->pixclock));
+	if (var->xres > MAX_XRES || var->yres > MAX_YRES)
 		return -EINVAL;
+
+	/* If board code provides us with a list of available modes, make sure
+	 * we use one of them. Find the mode closest to the requested one. The
+	 * distance between two modes is defined as the size of the
+	 * non-overlapping parts of the two rectangles.
+	 */
+	for (i = 0; i < ch->cfg.num_cfg; ++i) {
+		const struct fb_videomode *mode = &ch->cfg.lcd_cfg[i];
+		unsigned int dist;
+
+		/* We can only round up. */
+		if (var->xres > mode->xres || var->yres > mode->yres)
+			continue;
+
+		dist = var->xres * var->yres + mode->xres * mode->yres
+		     - 2 * min(var->xres, mode->xres)
+			 * min(var->yres, mode->yres);
+
+		if (dist < best_dist) {
+			best_xres = mode->xres;
+			best_yres = mode->yres;
+			best_dist = dist;
+		}
 	}
 
-	/* only accept the forced_bpp for dual channel configurations */
-	if (p->forced_bpp && p->forced_bpp != var->bits_per_pixel)
+	/* If no available mode can be used, return an error. */
+	if (ch->cfg.num_cfg != 0) {
+		if (best_dist = (unsigned int)-1)
+			return -EINVAL;
+
+		var->xres = best_xres;
+		var->yres = best_yres;
+	}
+
+	/* Make sure the virtual resolution is at least as big as the visible
+	 * resolution.
+	 */
+	if (var->xres_virtual < var->xres)
+		var->xres_virtual = var->xres;
+	if (var->yres_virtual < var->yres)
+		var->yres_virtual = var->yres;
+
+	if (var->bits_per_pixel <= 16) {		/* RGB 565 */
+		var->bits_per_pixel = 16;
+		var->red.offset = 11;
+		var->red.length = 5;
+		var->green.offset = 5;
+		var->green.length = 6;
+		var->blue.offset = 0;
+		var->blue.length = 5;
+		var->transp.offset = 0;
+		var->transp.length = 0;
+	} else if (var->bits_per_pixel <= 24) {		/* RGB 888 */
+		var->bits_per_pixel = 24;
+		var->red.offset = 16;
+		var->red.length = 8;
+		var->green.offset = 8;
+		var->green.length = 8;
+		var->blue.offset = 0;
+		var->blue.length = 8;
+		var->transp.offset = 0;
+		var->transp.length = 0;
+	} else if (var->bits_per_pixel <= 32) {		/* RGBA 888 */
+		var->bits_per_pixel = 32;
+		var->red.offset = 16;
+		var->red.length = 8;
+		var->green.offset = 8;
+		var->green.length = 8;
+		var->blue.offset = 0;
+		var->blue.length = 8;
+		var->transp.offset = 24;
+		var->transp.length = 8;
+	} else
 		return -EINVAL;
 
-	switch (var->bits_per_pixel) {
-	case 16: /* PKF[4:0] = 00011 - RGB 565 */
-	case 24: /* PKF[4:0] = 01011 - RGB 888 */
-	case 32: /* PKF[4:0] = 00000 - RGBA 888 */
-		break;
-	default:
+	var->red.msb_right = 0;
+	var->green.msb_right = 0;
+	var->blue.msb_right = 0;
+	var->transp.msb_right = 0;
+
+	/* Make sure we don't exceed our allocated memory. */
+	if (var->xres_virtual * var->yres_virtual * var->bits_per_pixel / 8 >
+	    info->fix.smem_len)
+		return -EINVAL;
+
+	/* only accept the forced_bpp for dual channel configurations */
+	if (p->forced_bpp && p->forced_bpp != var->bits_per_pixel)
 		return -EINVAL;
-	}
 
 	return 0;
 }
-- 
1.7.3.4


^ permalink raw reply related

* [PATCH v2 3/8] fbdev: sh_mobile_lcdc: Add support for format changes at runtime
From: Laurent Pinchart @ 2011-08-31 11:00 UTC (permalink / raw)
  To: linux-fbdev

Implement .fb_set_par to support frame buffer format changes at runtime.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/video/sh_mobile_lcdcfb.c |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/drivers/video/sh_mobile_lcdcfb.c b/drivers/video/sh_mobile_lcdcfb.c
index 33b0ff8..f9f420d 100644
--- a/drivers/video/sh_mobile_lcdcfb.c
+++ b/drivers/video/sh_mobile_lcdcfb.c
@@ -1154,6 +1154,19 @@ static int sh_mobile_check_var(struct fb_var_screeninfo *var, struct fb_info *in
 	return 0;
 }
 
+static int sh_mobile_set_par(struct fb_info *info)
+{
+	struct sh_mobile_lcdc_chan *ch = info->par;
+	int ret;
+
+	sh_mobile_lcdc_stop(ch->lcdc);
+	ret = sh_mobile_lcdc_start(ch->lcdc);
+	if (ret < 0)
+		dev_err(info->dev, "%s: unable to restart LCDC\n", __func__);
+
+	return ret;
+}
+
 /*
  * Screen blanking. Behavior is as follows:
  * FB_BLANK_UNBLANK: screen unblanked, clocks enabled
@@ -1211,6 +1224,7 @@ static struct fb_ops sh_mobile_lcdc_ops = {
 	.fb_open	= sh_mobile_open,
 	.fb_release	= sh_mobile_release,
 	.fb_check_var	= sh_mobile_check_var,
+	.fb_set_par	= sh_mobile_set_par,
 };
 
 static int sh_mobile_lcdc_update_bl(struct backlight_device *bdev)
-- 
1.7.3.4


^ permalink raw reply related

* [PATCH v2 4/8] fbdev: sh_mobile_lcdc: use display information in info for panning
From: Laurent Pinchart @ 2011-08-31 11:00 UTC (permalink / raw)
  To: linux-fbdev

We must not use any information in the passed var besides xoffset,
yoffset and vmode as otherwise applications might abuse it.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/video/sh_mobile_lcdcfb.c |   24 ++++++++++++------------
 1 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/video/sh_mobile_lcdcfb.c b/drivers/video/sh_mobile_lcdcfb.c
index f9f420d..1ff215c 100644
--- a/drivers/video/sh_mobile_lcdcfb.c
+++ b/drivers/video/sh_mobile_lcdcfb.c
@@ -877,12 +877,12 @@ static int sh_mobile_fb_pan_display(struct fb_var_screeninfo *var,
 	unsigned long base_addr_y, base_addr_c;
 	unsigned long c_offset;
 
-	if (!var->nonstd)
-		new_pan_offset = (var->yoffset * info->fix.line_length) +
-			(var->xoffset * (info->var.bits_per_pixel / 8));
+	if (!info->var.nonstd)
+		new_pan_offset = var->yoffset * info->fix.line_length
+			       + var->xoffset * (info->var.bits_per_pixel / 8);
 	else
-		new_pan_offset = (var->yoffset * info->fix.line_length) +
-			(var->xoffset);
+		new_pan_offset = var->yoffset * info->fix.line_length
+			       + var->xoffset;
 
 	if (new_pan_offset = ch->pan_offset)
 		return 0;	/* No change, do nothing */
@@ -891,13 +891,13 @@ static int sh_mobile_fb_pan_display(struct fb_var_screeninfo *var,
 
 	/* Set the source address for the next refresh */
 	base_addr_y = ch->dma_handle + new_pan_offset;
-	if (var->nonstd) {
+	if (info->var.nonstd) {
 		/* Set y offset */
-		c_offset = (var->yoffset *
-			info->fix.line_length *
-			(info->var.bits_per_pixel - 8)) / 8;
-		base_addr_c = ch->dma_handle + var->xres * var->yres_virtual +
-			c_offset;
+		c_offset = var->yoffset * info->fix.line_length
+			 * (info->var.bits_per_pixel - 8) / 8;
+		base_addr_c = ch->dma_handle
+			    + info->var.xres * info->var.yres_virtual
+			    + c_offset;
 		/* Set x offset */
 		if (info->var.bits_per_pixel = 24)
 			base_addr_c += 2 * var->xoffset;
@@ -923,7 +923,7 @@ static int sh_mobile_fb_pan_display(struct fb_var_screeninfo *var,
 	ch->base_addr_c = base_addr_c;
 
 	lcdc_write_chan_mirror(ch, LDSA1R, base_addr_y);
-	if (var->nonstd)
+	if (info->var.nonstd)
 		lcdc_write_chan_mirror(ch, LDSA2R, base_addr_c);
 
 	if (lcdc_chan_is_sublcd(ch))
-- 
1.7.3.4


^ permalink raw reply related

* [PATCH v2 5/8] fbdev: sh_mobile_lcdc: Update fix.line_length in .fb_set_par()
From: Laurent Pinchart @ 2011-08-31 11:00 UTC (permalink / raw)
  To: linux-fbdev

Instead of updating the fixed screen information line length manually
after calling fb_set_var() in sh_mobile_fb_reconfig(), update the field
in the .fb_set_par() operation handler.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/video/sh_mobile_lcdcfb.c |   17 +++++++++++------
 1 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/video/sh_mobile_lcdcfb.c b/drivers/video/sh_mobile_lcdcfb.c
index 1ff215c..b6da1d6 100644
--- a/drivers/video/sh_mobile_lcdcfb.c
+++ b/drivers/video/sh_mobile_lcdcfb.c
@@ -998,11 +998,6 @@ static void sh_mobile_fb_reconfig(struct fb_info *info)
 		/* Couldn't reconfigure, hopefully, can continue as before */
 		return;
 
-	if (info->var.nonstd)
-		info->fix.line_length = mode1.xres;
-	else
-		info->fix.line_length = mode1.xres * (ch->cfg.bpp / 8);
-
 	/*
 	 * fb_set_var() calls the notifier change internally, only if
 	 * FBINFO_MISC_USEREVENT flag is set. Since we do not want to fake a
@@ -1157,12 +1152,22 @@ static int sh_mobile_check_var(struct fb_var_screeninfo *var, struct fb_info *in
 static int sh_mobile_set_par(struct fb_info *info)
 {
 	struct sh_mobile_lcdc_chan *ch = info->par;
+	u32 line_length = info->fix.line_length;
 	int ret;
 
 	sh_mobile_lcdc_stop(ch->lcdc);
+
+	if (info->var.nonstd)
+		info->fix.line_length = info->var.xres;
+	else
+		info->fix.line_length = info->var.xres
+				      * info->var.bits_per_pixel / 8;
+
 	ret = sh_mobile_lcdc_start(ch->lcdc);
-	if (ret < 0)
+	if (ret < 0) {
 		dev_err(info->dev, "%s: unable to restart LCDC\n", __func__);
+		info->fix.line_length = line_length;
+	}
 
 	return ret;
 }
-- 
1.7.3.4


^ permalink raw reply related

* [PATCH v2 6/8] fbdev: sh_mobile_lcdc: Avoid forward declarations
From: Laurent Pinchart @ 2011-08-31 11:00 UTC (permalink / raw)
  To: linux-fbdev

Reorder probe/remove functions to avoid forward declarations.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/video/sh_mobile_lcdcfb.c |  102 ++++++++++++++++++-------------------
 1 files changed, 50 insertions(+), 52 deletions(-)

diff --git a/drivers/video/sh_mobile_lcdcfb.c b/drivers/video/sh_mobile_lcdcfb.c
index b6da1d6..366315b 100644
--- a/drivers/video/sh_mobile_lcdcfb.c
+++ b/drivers/video/sh_mobile_lcdcfb.c
@@ -1434,7 +1434,56 @@ static int sh_mobile_lcdc_notify(struct notifier_block *nb,
 	return NOTIFY_OK;
 }
 
-static int sh_mobile_lcdc_remove(struct platform_device *pdev);
+static int sh_mobile_lcdc_remove(struct platform_device *pdev)
+{
+	struct sh_mobile_lcdc_priv *priv = platform_get_drvdata(pdev);
+	struct fb_info *info;
+	int i;
+
+	fb_unregister_client(&priv->notifier);
+
+	for (i = 0; i < ARRAY_SIZE(priv->ch); i++)
+		if (priv->ch[i].info && priv->ch[i].info->dev)
+			unregister_framebuffer(priv->ch[i].info);
+
+	sh_mobile_lcdc_stop(priv);
+
+	for (i = 0; i < ARRAY_SIZE(priv->ch); i++) {
+		info = priv->ch[i].info;
+
+		if (!info || !info->device)
+			continue;
+
+		if (priv->ch[i].sglist)
+			vfree(priv->ch[i].sglist);
+
+		if (info->screen_base)
+			dma_free_coherent(&pdev->dev, info->fix.smem_len,
+					  info->screen_base,
+					  priv->ch[i].dma_handle);
+		fb_dealloc_cmap(&info->cmap);
+		framebuffer_release(info);
+	}
+
+	for (i = 0; i < ARRAY_SIZE(priv->ch); i++) {
+		if (priv->ch[i].bl)
+			sh_mobile_lcdc_bl_remove(priv->ch[i].bl);
+	}
+
+	if (priv->dot_clk)
+		clk_put(priv->dot_clk);
+
+	if (priv->dev)
+		pm_runtime_disable(priv->dev);
+
+	if (priv->base)
+		iounmap(priv->base);
+
+	if (priv->irq)
+		free_irq(priv->irq, priv);
+	kfree(priv);
+	return 0;
+}
 
 static int __devinit sh_mobile_lcdc_probe(struct platform_device *pdev)
 {
@@ -1691,57 +1740,6 @@ err1:
 	return error;
 }
 
-static int sh_mobile_lcdc_remove(struct platform_device *pdev)
-{
-	struct sh_mobile_lcdc_priv *priv = platform_get_drvdata(pdev);
-	struct fb_info *info;
-	int i;
-
-	fb_unregister_client(&priv->notifier);
-
-	for (i = 0; i < ARRAY_SIZE(priv->ch); i++)
-		if (priv->ch[i].info && priv->ch[i].info->dev)
-			unregister_framebuffer(priv->ch[i].info);
-
-	sh_mobile_lcdc_stop(priv);
-
-	for (i = 0; i < ARRAY_SIZE(priv->ch); i++) {
-		info = priv->ch[i].info;
-
-		if (!info || !info->device)
-			continue;
-
-		if (priv->ch[i].sglist)
-			vfree(priv->ch[i].sglist);
-
-		if (info->screen_base)
-			dma_free_coherent(&pdev->dev, info->fix.smem_len,
-					  info->screen_base,
-					  priv->ch[i].dma_handle);
-		fb_dealloc_cmap(&info->cmap);
-		framebuffer_release(info);
-	}
-
-	for (i = 0; i < ARRAY_SIZE(priv->ch); i++) {
-		if (priv->ch[i].bl)
-			sh_mobile_lcdc_bl_remove(priv->ch[i].bl);
-	}
-
-	if (priv->dot_clk)
-		clk_put(priv->dot_clk);
-
-	if (priv->dev)
-		pm_runtime_disable(priv->dev);
-
-	if (priv->base)
-		iounmap(priv->base);
-
-	if (priv->irq)
-		free_irq(priv->irq, priv);
-	kfree(priv);
-	return 0;
-}
-
 static struct platform_driver sh_mobile_lcdc_driver = {
 	.driver		= {
 		.name		= "sh_mobile_lcdc_fb",
-- 
1.7.3.4


^ permalink raw reply related

* [PATCH v2 7/8] fbdev: sh_mobile_lcdc: Split channel initialization from probe function
From: Laurent Pinchart @ 2011-08-31 11:00 UTC (permalink / raw)
  To: linux-fbdev

Move channel initialization to sh_mobile_lcdc_channel_init() and call
the function from sh_mobile_lcdc_probe(). This makes the code more
readable and prepares it for fix/var initialization rework.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/video/sh_mobile_lcdcfb.c |  247 ++++++++++++++++++++------------------
 1 files changed, 129 insertions(+), 118 deletions(-)

diff --git a/drivers/video/sh_mobile_lcdcfb.c b/drivers/video/sh_mobile_lcdcfb.c
index 366315b..d1576e2 100644
--- a/drivers/video/sh_mobile_lcdcfb.c
+++ b/drivers/video/sh_mobile_lcdcfb.c
@@ -1485,15 +1485,129 @@ static int sh_mobile_lcdc_remove(struct platform_device *pdev)
 	return 0;
 }
 
-static int __devinit sh_mobile_lcdc_probe(struct platform_device *pdev)
+static int __devinit sh_mobile_lcdc_channel_init(struct sh_mobile_lcdc_chan *ch,
+						 struct device *dev)
 {
+	struct sh_mobile_lcdc_chan_cfg *cfg = &ch->cfg;
+	const struct fb_videomode *max_mode;
+	const struct fb_videomode *mode;
+	struct fb_var_screeninfo *var;
 	struct fb_info *info;
-	struct sh_mobile_lcdc_priv *priv;
+	unsigned int max_size;
+	int num_cfg;
+	void *buf;
+	int ret;
+	int i;
+
+	ch->info = framebuffer_alloc(0, dev);
+	if (!ch->info) {
+		dev_err(dev, "unable to allocate fb_info\n");
+		return -ENOMEM;
+	}
+
+	info = ch->info;
+	var = &info->var;
+	info->fbops = &sh_mobile_lcdc_ops;
+	info->par = ch;
+
+	mutex_init(&ch->open_lock);
+
+	/* Iterate through the modes to validate them and find the highest
+	 * resolution.
+	 */
+	max_mode = NULL;
+	max_size = 0;
+
+	for (i = 0, mode = cfg->lcd_cfg; i < cfg->num_cfg; i++, mode++) {
+		unsigned int size = mode->yres * mode->xres;
+
+		/* NV12 buffers must have even number of lines */
+		if ((cfg->nonstd) && cfg->bpp = 12 &&
+				(mode->yres & 0x1)) {
+			dev_err(dev, "yres must be multiple of 2 for YCbCr420 "
+				"mode.\n");
+			return -EINVAL;
+		}
+
+		if (size > max_size) {
+			max_mode = mode;
+			max_size = size;
+		}
+	}
+
+	if (!max_size)
+		max_size = MAX_XRES * MAX_YRES;
+	else
+		dev_dbg(dev, "Found largest videomode %ux%u\n",
+			max_mode->xres, max_mode->yres);
+
+	info->fix = sh_mobile_lcdc_fix;
+	info->fix.smem_len = max_size * 2 * cfg->bpp / 8;
+
+	 /* Only pan in 2 line steps for NV12 */
+	if (cfg->nonstd && cfg->bpp = 12)
+		info->fix.ypanstep = 2;
+
+	if (cfg->lcd_cfg = NULL) {
+		mode = &default_720p;
+		num_cfg = 1;
+	} else {
+		mode = cfg->lcd_cfg;
+		num_cfg = cfg->num_cfg;
+	}
+
+	fb_videomode_to_modelist(mode, num_cfg, &info->modelist);
+
+	fb_videomode_to_var(var, mode);
+	var->width = cfg->lcd_size_cfg.width;
+	var->height = cfg->lcd_size_cfg.height;
+	/* Default Y virtual resolution is 2x panel size */
+	var->yres_virtual = var->yres * 2;
+	var->activate = FB_ACTIVATE_NOW;
+
+	ret = sh_mobile_lcdc_set_bpp(var, cfg->bpp, cfg->nonstd);
+	if (ret)
+		return ret;
+
+	buf = dma_alloc_coherent(dev, info->fix.smem_len, &ch->dma_handle,
+				 GFP_KERNEL);
+	if (!buf) {
+		dev_err(dev, "unable to allocate buffer\n");
+		return -ENOMEM;
+	}
+
+	info->pseudo_palette = &ch->pseudo_palette;
+	info->flags = FBINFO_FLAG_DEFAULT;
+
+	ret = fb_alloc_cmap(&info->cmap, PALETTE_NR, 0);
+	if (ret < 0) {
+		dev_err(dev, "unable to allocate cmap\n");
+		dma_free_coherent(dev, info->fix.smem_len,
+				  buf, ch->dma_handle);
+		return ret;
+	}
+
+	info->fix.smem_start = ch->dma_handle;
+	if (var->nonstd)
+		info->fix.line_length = var->xres;
+	else
+		info->fix.line_length = var->xres * (cfg->bpp / 8);
+
+	info->screen_base = buf;
+	info->device = dev;
+	ch->display_var = *var;
+
+	return 0;
+}
+
+static int __devinit sh_mobile_lcdc_probe(struct platform_device *pdev)
+{
 	struct sh_mobile_lcdc_info *pdata = pdev->dev.platform_data;
+	struct sh_mobile_lcdc_priv *priv;
 	struct resource *res;
+	int num_channels;
 	int error;
-	void *buf;
-	int i, j;
+	int i;
 
 	if (!pdata) {
 		dev_err(&pdev->dev, "no platform data defined\n");
@@ -1525,9 +1639,8 @@ static int __devinit sh_mobile_lcdc_probe(struct platform_device *pdev)
 	priv->irq = i;
 	atomic_set(&priv->hw_usecnt, -1);
 
-	j = 0;
-	for (i = 0; i < ARRAY_SIZE(pdata->ch); i++) {
-		struct sh_mobile_lcdc_chan *ch = priv->ch + j;
+	for (i = 0, num_channels = 0; i < ARRAY_SIZE(pdata->ch); i++) {
+		struct sh_mobile_lcdc_chan *ch = priv->ch + num_channels;
 
 		ch->lcdc = priv;
 		memcpy(&ch->cfg, &pdata->ch[i], sizeof(pdata->ch[i]));
@@ -1549,24 +1662,24 @@ static int __devinit sh_mobile_lcdc_probe(struct platform_device *pdev)
 		case LCDC_CHAN_MAINLCD:
 			ch->enabled = LDCNT2R_ME;
 			ch->reg_offs = lcdc_offs_mainlcd;
-			j++;
+			num_channels++;
 			break;
 		case LCDC_CHAN_SUBLCD:
 			ch->enabled = LDCNT2R_SE;
 			ch->reg_offs = lcdc_offs_sublcd;
-			j++;
+			num_channels++;
 			break;
 		}
 	}
 
-	if (!j) {
+	if (!num_channels) {
 		dev_err(&pdev->dev, "no channels defined\n");
 		error = -EINVAL;
 		goto err1;
 	}
 
 	/* for dual channel LCDC (MAIN + SUB) force shared bpp setting */
-	if (j = 2)
+	if (num_channels = 2)
 		priv->forced_bpp = pdata->ch[0].bpp;
 
 	priv->base = ioremap_nocache(res->start, resource_size(res));
@@ -1581,125 +1694,23 @@ static int __devinit sh_mobile_lcdc_probe(struct platform_device *pdev)
 
 	priv->meram_dev = pdata->meram_dev;
 
-	for (i = 0; i < j; i++) {
-		struct fb_var_screeninfo *var;
-		const struct fb_videomode *lcd_cfg, *max_cfg = NULL;
+	for (i = 0; i < num_channels; i++) {
 		struct sh_mobile_lcdc_chan *ch = priv->ch + i;
-		struct sh_mobile_lcdc_chan_cfg *cfg = &ch->cfg;
-		const struct fb_videomode *mode = cfg->lcd_cfg;
-		unsigned long max_size = 0;
-		int k;
-		int num_cfg;
-
-		ch->info = framebuffer_alloc(0, &pdev->dev);
-		if (!ch->info) {
-			dev_err(&pdev->dev, "unable to allocate fb_info\n");
-			error = -ENOMEM;
-			break;
-		}
-
-		info = ch->info;
-		var = &info->var;
-		info->fbops = &sh_mobile_lcdc_ops;
-		info->par = ch;
-
-		mutex_init(&ch->open_lock);
-
-		for (k = 0, lcd_cfg = mode;
-		     k < cfg->num_cfg && lcd_cfg;
-		     k++, lcd_cfg++) {
-			unsigned long size = lcd_cfg->yres * lcd_cfg->xres;
-			/* NV12 buffers must have even number of lines */
-			if ((cfg->nonstd) && cfg->bpp = 12 &&
-					(lcd_cfg->yres & 0x1)) {
-				dev_err(&pdev->dev, "yres must be multiple of 2"
-						" for YCbCr420 mode.\n");
-				error = -EINVAL;
-				goto err1;
-			}
-
-			if (size > max_size) {
-				max_cfg = lcd_cfg;
-				max_size = size;
-			}
-		}
-
-		if (!mode)
-			max_size = MAX_XRES * MAX_YRES;
-		else if (max_cfg)
-			dev_dbg(&pdev->dev, "Found largest videomode %ux%u\n",
-				max_cfg->xres, max_cfg->yres);
-
-		info->fix = sh_mobile_lcdc_fix;
-		info->fix.smem_len = max_size * 2 * cfg->bpp / 8;
-
-		 /* Only pan in 2 line steps for NV12 */
-		if (cfg->nonstd && cfg->bpp = 12)
-			info->fix.ypanstep = 2;
-
-		if (!mode) {
-			mode = &default_720p;
-			num_cfg = 1;
-		} else {
-			num_cfg = cfg->num_cfg;
-		}
-
-		fb_videomode_to_modelist(mode, num_cfg, &info->modelist);
 
-		fb_videomode_to_var(var, mode);
-		var->width = cfg->lcd_size_cfg.width;
-		var->height = cfg->lcd_size_cfg.height;
-		/* Default Y virtual resolution is 2x panel size */
-		var->yres_virtual = var->yres * 2;
-		var->activate = FB_ACTIVATE_NOW;
-
-		error = sh_mobile_lcdc_set_bpp(var, cfg->bpp, cfg->nonstd);
+		error = sh_mobile_lcdc_channel_init(ch, &pdev->dev);
 		if (error)
-			break;
-
-		buf = dma_alloc_coherent(&pdev->dev, info->fix.smem_len,
-					 &ch->dma_handle, GFP_KERNEL);
-		if (!buf) {
-			dev_err(&pdev->dev, "unable to allocate buffer\n");
-			error = -ENOMEM;
-			break;
-		}
-
-		info->pseudo_palette = &ch->pseudo_palette;
-		info->flags = FBINFO_FLAG_DEFAULT;
-
-		error = fb_alloc_cmap(&info->cmap, PALETTE_NR, 0);
-		if (error < 0) {
-			dev_err(&pdev->dev, "unable to allocate cmap\n");
-			dma_free_coherent(&pdev->dev, info->fix.smem_len,
-					  buf, ch->dma_handle);
-			break;
-		}
-
-		info->fix.smem_start = ch->dma_handle;
-		if (var->nonstd)
-			info->fix.line_length = var->xres;
-		else
-			info->fix.line_length = var->xres * (cfg->bpp / 8);
-
-		info->screen_base = buf;
-		info->device = &pdev->dev;
-		ch->display_var = *var;
+			goto err1;
 	}
 
-	if (error)
-		goto err1;
-
 	error = sh_mobile_lcdc_start(priv);
 	if (error) {
 		dev_err(&pdev->dev, "unable to start hardware\n");
 		goto err1;
 	}
 
-	for (i = 0; i < j; i++) {
+	for (i = 0; i < num_channels; i++) {
 		struct sh_mobile_lcdc_chan *ch = priv->ch + i;
-
-		info = ch->info;
+		struct fb_info *info = ch->info;
 
 		if (info->fbdefio) {
 			ch->sglist = vmalloc(sizeof(struct scatterlist) *
-- 
1.7.3.4


^ permalink raw reply related

* [PATCH v2 8/8] fbdev: sh_mobile_lcdc: Remove sh_mobile_lcdc_set_bpp()
From: Laurent Pinchart @ 2011-08-31 11:00 UTC (permalink / raw)
  To: linux-fbdev

The function duplicates code found in sh_mobile_check_var(). Remove
sh_mobile_lcdc_set_bpp() and call sh_mobile_check_var() instead.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/video/sh_mobile_lcdcfb.c |   87 +++++++------------------------------
 1 files changed, 17 insertions(+), 70 deletions(-)

diff --git a/drivers/video/sh_mobile_lcdcfb.c b/drivers/video/sh_mobile_lcdcfb.c
index d1576e2..97ab8ba 100644
--- a/drivers/video/sh_mobile_lcdcfb.c
+++ b/drivers/video/sh_mobile_lcdcfb.c
@@ -1291,66 +1291,6 @@ static void sh_mobile_lcdc_bl_remove(struct backlight_device *bdev)
 	backlight_device_unregister(bdev);
 }
 
-static int sh_mobile_lcdc_set_bpp(struct fb_var_screeninfo *var, int bpp,
-				   int nonstd)
-{
-	if (nonstd) {
-		switch (bpp) {
-		case 12:
-		case 16:
-		case 24:
-			var->bits_per_pixel = bpp;
-			var->nonstd = nonstd;
-			return 0;
-		default:
-			return -EINVAL;
-		}
-	}
-
-	switch (bpp) {
-	case 16: /* PKF[4:0] = 00011 - RGB 565 */
-		var->red.offset = 11;
-		var->red.length = 5;
-		var->green.offset = 5;
-		var->green.length = 6;
-		var->blue.offset = 0;
-		var->blue.length = 5;
-		var->transp.offset = 0;
-		var->transp.length = 0;
-		break;
-
-	case 24: /* PKF[4:0] = 01011 - RGB 888 */
-		var->red.offset = 16;
-		var->red.length = 8;
-		var->green.offset = 8;
-		var->green.length = 8;
-		var->blue.offset = 0;
-		var->blue.length = 8;
-		var->transp.offset = 0;
-		var->transp.length = 0;
-		break;
-
-	case 32: /* PKF[4:0] = 00000 - RGBA 888 */
-		var->red.offset = 16;
-		var->red.length = 8;
-		var->green.offset = 8;
-		var->green.length = 8;
-		var->blue.offset = 0;
-		var->blue.length = 8;
-		var->transp.offset = 24;
-		var->transp.length = 8;
-		break;
-	default:
-		return -EINVAL;
-	}
-	var->bits_per_pixel = bpp;
-	var->red.msb_right = 0;
-	var->green.msb_right = 0;
-	var->blue.msb_right = 0;
-	var->transp.msb_right = 0;
-	return 0;
-}
-
 static int sh_mobile_lcdc_suspend(struct device *dev)
 {
 	struct platform_device *pdev = to_platform_device(dev);
@@ -1499,6 +1439,9 @@ static int __devinit sh_mobile_lcdc_channel_init(struct sh_mobile_lcdc_chan *ch,
 	int ret;
 	int i;
 
+	mutex_init(&ch->open_lock);
+
+	/* Allocate the frame buffer device. */
 	ch->info = framebuffer_alloc(0, dev);
 	if (!ch->info) {
 		dev_err(dev, "unable to allocate fb_info\n");
@@ -1506,11 +1449,10 @@ static int __devinit sh_mobile_lcdc_channel_init(struct sh_mobile_lcdc_chan *ch,
 	}
 
 	info = ch->info;
-	var = &info->var;
 	info->fbops = &sh_mobile_lcdc_ops;
 	info->par = ch;
-
-	mutex_init(&ch->open_lock);
+	info->pseudo_palette = &ch->pseudo_palette;
+	info->flags = FBINFO_FLAG_DEFAULT;
 
 	/* Iterate through the modes to validate them and find the highest
 	 * resolution.
@@ -1541,13 +1483,15 @@ static int __devinit sh_mobile_lcdc_channel_init(struct sh_mobile_lcdc_chan *ch,
 		dev_dbg(dev, "Found largest videomode %ux%u\n",
 			max_mode->xres, max_mode->yres);
 
+	/* Initialize fixed screen information. Restrict pan to 2 lines steps
+	 * for NV12.
+	 */
 	info->fix = sh_mobile_lcdc_fix;
 	info->fix.smem_len = max_size * 2 * cfg->bpp / 8;
-
-	 /* Only pan in 2 line steps for NV12 */
 	if (cfg->nonstd && cfg->bpp = 12)
 		info->fix.ypanstep = 2;
 
+	/* Create the mode list. */
 	if (cfg->lcd_cfg = NULL) {
 		mode = &default_720p;
 		num_cfg = 1;
@@ -1558,17 +1502,23 @@ static int __devinit sh_mobile_lcdc_channel_init(struct sh_mobile_lcdc_chan *ch,
 
 	fb_videomode_to_modelist(mode, num_cfg, &info->modelist);
 
+	/* Initialize variable screen information using the first mode as
+	 * default. The default Y virtual resolution is twice the panel size to
+	 * allow for double-buffering.
+	 */
+	var = &info->var;
 	fb_videomode_to_var(var, mode);
+	var->bits_per_pixel = cfg->bpp;
 	var->width = cfg->lcd_size_cfg.width;
 	var->height = cfg->lcd_size_cfg.height;
-	/* Default Y virtual resolution is 2x panel size */
 	var->yres_virtual = var->yres * 2;
 	var->activate = FB_ACTIVATE_NOW;
 
-	ret = sh_mobile_lcdc_set_bpp(var, cfg->bpp, cfg->nonstd);
+	ret = sh_mobile_check_var(var, info);
 	if (ret)
 		return ret;
 
+	/* Allocate frame buffer memory and color map. */
 	buf = dma_alloc_coherent(dev, info->fix.smem_len, &ch->dma_handle,
 				 GFP_KERNEL);
 	if (!buf) {
@@ -1576,9 +1526,6 @@ static int __devinit sh_mobile_lcdc_channel_init(struct sh_mobile_lcdc_chan *ch,
 		return -ENOMEM;
 	}
 
-	info->pseudo_palette = &ch->pseudo_palette;
-	info->flags = FBINFO_FLAG_DEFAULT;
-
 	ret = fb_alloc_cmap(&info->cmap, PALETTE_NR, 0);
 	if (ret < 0) {
 		dev_err(dev, "unable to allocate cmap\n");
-- 
1.7.3.4


^ permalink raw reply related


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