RE: [PATCH 3/4] s3c-fb: Add support S5PV310 FIMD
-Original Message-
From: Paul Mundt [mailto:let...@linux-sh.org]
Sent: Friday, November 12, 2010 7:09 PM
To: Inki Dae
Cc: 'Kukjin Kim'; 'Sangbeom Kim'; linux-arm-ker...@lists.infradead.org;
linux-samsung-soc@vger.kernel.org; linux-fb...@vger.kernel.org; ben-
li...@fluff.org; a...@linux-foundation.org; 'Jonghun Han'
Subject: Re: [PATCH 3/4] s3c-fb: Add support S5PV310 FIMD
On Fri, Nov 12, 2010 at 06:54:29PM +0900, Inki Dae wrote:
@@ -1314,13 +1319,22 @@ static int __devinit s3c_fb_probe(struct
platform_device *pdev)
sfb-pdata = pd;
sfb-variant = fbdrv-variant;
- sfb-bus_clk = clk_get(dev, lcd);
- if (IS_ERR(sfb-bus_clk)) {
- dev_err(dev, failed to get bus clock\n);
+ /* if sclk_name is NULL then it would use bus clock as default. */
+ if (!pd-sclk_name)
+ sfb-lcd_clk = clk_get(dev, lcd);
+ else
+ sfb-lcd_clk = clk_get(dev, pd-sclk_name);
+
No, this is totally pointless. Simply establish a clk lookup for the
configuration you are running on in your board code
You mean, call the function at s3c_fb_probe() establishing clk lookup in
board code?
Sorry but I could understand. Please give me more detail.
and always have
clk_get() operate on a fixed id. You can use this same scheme with any
optional clocks you have too, and reconfigure according to whether the
lookups for optional clocks succeeded or not.
optional clocks could be sclk_name?
I will glad to show me some example for it.
thank you.
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] s3c-fb: Add support S5PV310 FIMD
Hi, Thanks for comment. [snip] I don't think that the driver is the right place to change the parent of the sclk_fimd clock. It should be done in the boot loader or the board startup code. As you know, there are two clock sources for LCD pixel clock. s3c-fb only used 'lcd' clock as a parent of LCD pixel clock. But from S5PV310 SCLK_FIMD is only used as a source of LCD pixel clock. And SCLK_FIMD is only for FIMD. So the parent of the SCLK_FIMD can be selected in the driver. In my opinion it doesn't matter. Parent clock is specific to machine because machines could have different type of lcd panel each other and any parent clocks for FIMD could be used according to the lcd panel. So for stable pixel clock setting, parent clock should be set at machine specific code and FIMD driver should preserve just clock gating. Right. Parent clock is specific to machine. But as you know, 'mout_mpll' can cover almost LCD pixel clock. If other parent clock is needed, I'll get the parent clock or separator via platform data from board file. It is better than setting parent clock in board file at the point of samsung mainline scheme. BRs, -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 3/4] s3c-fb: Add support S5PV310 FIMD
1AFAAVgAzADEAMAAgAEY
ASQBNAEQA
x-cr-puzzleid: {B7526269-A73C-412A-8DAE-B8EB603CF9D0}
Hi,
Marek Szyprowski wrote:
-Original Message-
From: Marek Szyprowski [mailto:m.szyprow...@samsung.com]
Sent: Tuesday, October 19, 2010 4:22 PM
To: 'Sangbeom Kim'; linux-arm-ker...@lists.infradead.org; linux-samsung-
s...@vger.kernel.org; linux-fb...@vger.kernel.org
Cc: 'Jonghun Han'; a...@linux-foundation.org; kgene@samsung.com; ben-
li...@fluff.org; kyungmin.p...@samsung.com
Subject: RE: [PATCH 3/4] s3c-fb: Add support S5PV310 FIMD
1AFAAVgAzADEAMAAgAEY
ASQBNAEQA
x-cr-puzzleid: {C4CCB40E-D5C6-4119-AA7C-BBBA7A1503E4}
Hello,
On Monday, October 18, 2010 2:55 PM Sangbeom Kim wrote:
From: Jonghun Han jonghun@samsung.com
This patch adds s3c_fb_driverdata for S5PV310 FIMD0. The clk_type is
added
to distinguish clock type in structure s3c_fb_variant and lcd_clk is
added
in structure s3c_fb to calculate divider for lcd panel.
FIMD can handles two clocks. The one is for FIMD IP and the other is for
LCD pixel clock. Before S5PV310 LCD pixel clock can be same with FIMD IP
clock. From S5PV310 LCD pixel clock is separated from FIMD IP clock.
Signed-off-by: Jonghun Han jonghun@samsung.com
Reviewed-by: Kukjin Kim kgene@samsung.com
Signed-off-by: Sangbeom Kim sbki...@samsung.com
Cc: Ben Dooks ben-li...@fluff.org
---
NOTE: This patch is only for FIMD0.
FIMD1 will be implemented later.
drivers/video/Kconfig |2 +-
drivers/video/s3c-fb.c | 128
++--
2 files changed, 114 insertions(+), 16 deletions(-)
[snip]
/* write the buffer address */
@@ -1286,8 +1292,10 @@ static int __devinit s3c_fb_probe(struct
platform_device *pdev)
struct s3c_fb_platdata *pd;
struct s3c_fb *sfb;
struct resource *res;
+ struct clk *mout_mpll = NULL;
int win;
int ret = 0;
+ u32 rate = 13400;
fbdrv = (struct s3c_fb_driverdata *)platform_get_device_id(pdev)-
driver_data;
@@ -1314,19 +1322,56 @@ static int __devinit s3c_fb_probe(struct
platform_device *pdev)
sfb-pdata = pd;
sfb-variant = fbdrv-variant;
- sfb-bus_clk = clk_get(dev, lcd);
- if (IS_ERR(sfb-bus_clk)) {
- dev_err(dev, failed to get bus clock\n);
+ switch (sfb-variant.clk_type) {
+ case FIMD_CLK_TYPE0:
+ sfb-bus_clk = clk_get(dev, lcd);
+ if (IS_ERR(sfb-bus_clk)) {
+ dev_err(dev, failed to get bus clock\n);
+ goto err_sfb;
+ }
+
+ clk_enable(sfb-bus_clk);
+
+ sfb-lcd_clk = sfb-bus_clk;
+ break;
+
+ case FIMD_CLK_TYPE1:
+ sfb-bus_clk = clk_get(pdev-dev, fimd);
+ if (IS_ERR(sfb-bus_clk)) {
+ dev_err(pdev-dev, failed to get clock for
fimd\n);
+ goto err_sfb;
+ }
+ clk_enable(sfb-bus_clk);
+
+ sfb-lcd_clk = clk_get(pdev-dev, sclk_fimd);
+ if (IS_ERR(sfb-lcd_clk)) {
+ dev_err(pdev-dev, failed to get sclk for
fimd\n);
+ goto err_bus_clk;
+ }
+
+ mout_mpll = clk_get(pdev-dev, mout_mpll);
+ if (IS_ERR(mout_mpll)) {
+ dev_err(pdev-dev, failed to get mout_mpll\n);
+ goto err_lcd_clk;
+ }
+ clk_set_parent(sfb-lcd_clk, mout_mpll);
+ clk_put(mout_mpll);
I don't think that the driver is the right place to change the parent of
the sclk_fimd
clock. It should be done in the boot loader or the board startup code.
As you know, there are two clock sources for LCD pixel clock.
s3c-fb only used 'lcd' clock as a parent of LCD pixel clock.
But from S5PV310 SCLK_FIMD is only used as a source of LCD pixel clock.
And SCLK_FIMD is only for FIMD. So the parent of the SCLK_FIMD can be
selected in the driver.
In my opinion it doesn't matter.
We should also be a bit more consistent on clock naming. s3c6410..s5pv210
used the 'lcd'
clock name. Maybe you should also provide a patch to rename all these
clocks to common
name.
Please refer to the following diagram.
Before S5PV310 clk 'lcd' was used for FIMD IP core clock and source of the
LCD pixel clock.
But the mux used to select source of LCD pixel clock is removed.
So 'lcd' clock is only used for core clock of FIMD IP. It isn't used for LCD
pixel clock.
As a result clock name was changed from lcd to fimd in the S5PV310
datasheet.
I agree to rearrange clock name later.
Before S5PV310
dsys bus
+---
|
|1.clk 'lcd'
|
| FIMD block
+---+---+
4.mout_mpll
RE: [PATCH 3/4] s3c-fb: Add support S5PV310 FIMD
Hello, Jonghun.
Below is my opinion.
Have a good time :)
-Original Message-
From: linux-fbdev-ow...@vger.kernel.org [mailto:linux-fbdev-
ow...@vger.kernel.org] On Behalf Of Jonghun Han
Sent: Thursday, October 21, 2010 12:45 PM
To: 'Marek Szyprowski'; 'Sangbeom Kim'; linux-arm-
ker...@lists.infradead.org; linux-samsung-soc@vger.kernel.org; linux-
fb...@vger.kernel.org
Cc: a...@linux-foundation.org; kgene@samsung.com; ben-li...@fluff.org;
kyungmin.p...@samsung.com
Subject: RE: [PATCH 3/4] s3c-fb: Add support S5PV310 FIMD
1AFAAVgAzADEAMAAgAEY
ASQBNAEQA
x-cr-puzzleid: {B7526269-A73C-412A-8DAE-B8EB603CF9D0}
Hi,
Marek Szyprowski wrote:
-Original Message-
From: Marek Szyprowski [mailto:m.szyprow...@samsung.com]
Sent: Tuesday, October 19, 2010 4:22 PM
To: 'Sangbeom Kim'; linux-arm-ker...@lists.infradead.org; linux-samsung-
s...@vger.kernel.org; linux-fb...@vger.kernel.org
Cc: 'Jonghun Han'; a...@linux-foundation.org; kgene@samsung.com;
ben-
li...@fluff.org; kyungmin.p...@samsung.com
Subject: RE: [PATCH 3/4] s3c-fb: Add support S5PV310 FIMD
1AFAAVgAzADEAMAAgAEY
ASQBNAEQA
x-cr-puzzleid: {C4CCB40E-D5C6-4119-AA7C-BBBA7A1503E4}
Hello,
On Monday, October 18, 2010 2:55 PM Sangbeom Kim wrote:
From: Jonghun Han jonghun@samsung.com
This patch adds s3c_fb_driverdata for S5PV310 FIMD0. The clk_type is
added
to distinguish clock type in structure s3c_fb_variant and lcd_clk is
added
in structure s3c_fb to calculate divider for lcd panel.
FIMD can handles two clocks. The one is for FIMD IP and the other is
for
LCD pixel clock. Before S5PV310 LCD pixel clock can be same with FIMD
IP
clock. From S5PV310 LCD pixel clock is separated from FIMD IP clock.
Signed-off-by: Jonghun Han jonghun@samsung.com
Reviewed-by: Kukjin Kim kgene@samsung.com
Signed-off-by: Sangbeom Kim sbki...@samsung.com
Cc: Ben Dooks ben-li...@fluff.org
---
NOTE: This patch is only for FIMD0.
FIMD1 will be implemented later.
drivers/video/Kconfig |2 +-
drivers/video/s3c-fb.c | 128
++--
2 files changed, 114 insertions(+), 16 deletions(-)
[snip]
/* write the buffer address */
@@ -1286,8 +1292,10 @@ static int __devinit s3c_fb_probe(struct
platform_device *pdev)
struct s3c_fb_platdata *pd;
struct s3c_fb *sfb;
struct resource *res;
+ struct clk *mout_mpll = NULL;
int win;
int ret = 0;
+ u32 rate = 13400;
fbdrv = (struct s3c_fb_driverdata *)platform_get_device_id(pdev)-
driver_data;
@@ -1314,19 +1322,56 @@ static int __devinit s3c_fb_probe(struct
platform_device *pdev)
sfb-pdata = pd;
sfb-variant = fbdrv-variant;
- sfb-bus_clk = clk_get(dev, lcd);
- if (IS_ERR(sfb-bus_clk)) {
- dev_err(dev, failed to get bus clock\n);
+ switch (sfb-variant.clk_type) {
+ case FIMD_CLK_TYPE0:
+ sfb-bus_clk = clk_get(dev, lcd);
+ if (IS_ERR(sfb-bus_clk)) {
+ dev_err(dev, failed to get bus clock\n);
+ goto err_sfb;
+ }
+
+ clk_enable(sfb-bus_clk);
+
+ sfb-lcd_clk = sfb-bus_clk;
+ break;
+
+ case FIMD_CLK_TYPE1:
+ sfb-bus_clk = clk_get(pdev-dev, fimd);
+ if (IS_ERR(sfb-bus_clk)) {
+ dev_err(pdev-dev, failed to get clock for
fimd\n);
+ goto err_sfb;
+ }
+ clk_enable(sfb-bus_clk);
+
+ sfb-lcd_clk = clk_get(pdev-dev, sclk_fimd);
+ if (IS_ERR(sfb-lcd_clk)) {
+ dev_err(pdev-dev, failed to get sclk for
fimd\n);
+ goto err_bus_clk;
+ }
+
+ mout_mpll = clk_get(pdev-dev, mout_mpll);
+ if (IS_ERR(mout_mpll)) {
+ dev_err(pdev-dev, failed to get mout_mpll\n);
+ goto err_lcd_clk;
+ }
+ clk_set_parent(sfb-lcd_clk, mout_mpll);
+ clk_put(mout_mpll);
I don't think that the driver is the right place to change the parent of
the sclk_fimd
clock. It should be done in the boot loader or the board startup code.
As you know, there are two clock sources for LCD pixel clock.
s3c-fb only used 'lcd' clock as a parent of LCD pixel clock.
But from S5PV310 SCLK_FIMD is only used as a source of LCD pixel clock.
And SCLK_FIMD is only for FIMD. So the parent of the SCLK_FIMD can be
selected in the driver.
In my opinion it doesn't matter.
Parent clock is specific to machine
because machines could have different type of lcd panel each other and
any parent clocks for FIMD could be used according to the lcd panel.
So for stable pixel clock setting, parent clock should be set
at machine specific code and FIMD driver should preserve just clock gating.
We should also be a bit more consistent on clock naming
RE: [PATCH 3/4] s3c-fb: Add support S5PV310 FIMD
1AFAAVgAzADEAMAAgAEY
ASQBNAEQA
x-cr-puzzleid: {C4CCB40E-D5C6-4119-AA7C-BBBA7A1503E4}
Hello,
On Monday, October 18, 2010 2:55 PM Sangbeom Kim wrote:
From: Jonghun Han jonghun@samsung.com
This patch adds s3c_fb_driverdata for S5PV310 FIMD0. The clk_type is added
to distinguish clock type in structure s3c_fb_variant and lcd_clk is added
in structure s3c_fb to calculate divider for lcd panel.
FIMD can handles two clocks. The one is for FIMD IP and the other is for
LCD pixel clock. Before S5PV310 LCD pixel clock can be same with FIMD IP
clock. From S5PV310 LCD pixel clock is separated from FIMD IP clock.
Signed-off-by: Jonghun Han jonghun@samsung.com
Reviewed-by: Kukjin Kim kgene@samsung.com
Signed-off-by: Sangbeom Kim sbki...@samsung.com
Cc: Ben Dooks ben-li...@fluff.org
---
NOTE: This patch is only for FIMD0.
FIMD1 will be implemented later.
drivers/video/Kconfig |2 +-
drivers/video/s3c-fb.c | 128
++--
2 files changed, 114 insertions(+), 16 deletions(-)
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 8b31fdf..3e2e02a 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -1946,7 +1946,7 @@ config FB_TMIO_ACCELL
config FB_S3C
tristate Samsung S3C framebuffer support
- depends on FB S3C_DEV_FB
+ depends on FB (S3C_DEV_FB || S5P_DEV_FIMD0)
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
diff --git a/drivers/video/s3c-fb.c b/drivers/video/s3c-fb.c
index f9aca9d..bc182ea 100644
--- a/drivers/video/s3c-fb.c
+++ b/drivers/video/s3c-fb.c
@@ -65,6 +65,9 @@ struct s3c_fb;
#define VIDOSD_C(win, variant) (OSD_BASE(win, variant) + 0x08)
#define VIDOSD_D(win, variant) (OSD_BASE(win, variant) + 0x0C)
+#define FIMD_CLK_TYPE0 0
+#define FIMD_CLK_TYPE1 1
+
/**
* struct s3c_fb_variant - fb variant information
* @is_2443: Set if S3C2443/S3C2416 style hardware.
@@ -97,6 +100,7 @@ struct s3c_fb_variant {
unsigned inthas_prtcon:1;
unsigned inthas_shadowcon:1;
+ unsigned intclk_type:1;
};
/**
@@ -183,7 +187,8 @@ struct s3c_fb_vsync {
* struct s3c_fb - overall hardware state of the hardware
* @dev: The device that we bound to, for printing, etc.
* @regs_res: The resource we claimed for the IO registers.
- * @bus_clk: The clk (hclk) feeding our interface and possibly pixclk.
+ * @bus_clk: The clk (hclk) feeding FIMD IP core.
+ * @lcd_clk: The clk (sclk) feeding our interface and possibly pixclk.
* @regs: The mapped hardware registers.
* @variant: Variant information for this hardware.
* @enabled: A bitmask of enabled hardware windows.
@@ -197,6 +202,7 @@ struct s3c_fb {
struct device *dev;
struct resource *regs_res;
struct clk *bus_clk;
+ struct clk *lcd_clk;
void __iomem*regs;
struct s3c_fb_variantvariant;
@@ -334,7 +340,7 @@ static int s3c_fb_check_var(struct fb_var_screeninfo *var,
*/
static int s3c_fb_calc_pixclk(struct s3c_fb *sfb, unsigned int pixclk)
{
- unsigned long clk = clk_get_rate(sfb-bus_clk);
+ unsigned long clk = clk_get_rate(sfb-lcd_clk);
unsigned long long tmp;
unsigned int result;
@@ -517,7 +523,7 @@ static int s3c_fb_set_par(struct fb_info *info)
data = VIDTCON2_LINEVAL(var-yres - 1) |
VIDTCON2_HOZVAL(var-xres - 1);
- writel(data, regs +sfb-variant.vidtcon + 8 );
+ writel(data, regs + sfb-variant.vidtcon + 8);
}
/* write the buffer address */
@@ -1286,8 +1292,10 @@ static int __devinit s3c_fb_probe(struct
platform_device *pdev)
struct s3c_fb_platdata *pd;
struct s3c_fb *sfb;
struct resource *res;
+ struct clk *mout_mpll = NULL;
int win;
int ret = 0;
+ u32 rate = 13400;
fbdrv = (struct s3c_fb_driverdata
*)platform_get_device_id(pdev)-driver_data;
@@ -1314,19 +1322,56 @@ static int __devinit s3c_fb_probe(struct
platform_device *pdev)
sfb-pdata = pd;
sfb-variant = fbdrv-variant;
- sfb-bus_clk = clk_get(dev, lcd);
- if (IS_ERR(sfb-bus_clk)) {
- dev_err(dev, failed to get bus clock\n);
+ switch (sfb-variant.clk_type) {
+ case FIMD_CLK_TYPE0:
+ sfb-bus_clk = clk_get(dev, lcd);
+ if (IS_ERR(sfb-bus_clk)) {
+ dev_err(dev, failed to get bus clock\n);
+ goto err_sfb;
+ }
+
+ clk_enable(sfb-bus_clk);
+
+ sfb-lcd_clk = sfb-bus_clk;
+ break;
+
+ case FIMD_CLK_TYPE1:
+ sfb-bus_clk = clk_get(pdev-dev, fimd);
+ if (IS_ERR(sfb-bus_clk)) {
+ dev_err(pdev-dev, failed to get clock for fimd\n);
+ goto err_sfb;
+
