diff mbox

[1/6] OMAPDSS: DISPC: Remove cpu_is_xxxx checks

Message ID 5344e530a125ef5c5dfeb00e54b7d32df6169aa9.1343912532.git.cmahapatra@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Chandrabhanu Mahapatra Aug. 7, 2012, 8:27 a.m. UTC
The cpu_is checks have been removed from DISPC providing it a much generic and
cleaner interface. The OMAP version and revision specific functions are
initialized by dispc_ops structure in dss features.

Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
---
 drivers/video/omap2/dss/dispc.c        |  386 +++++++++++++++++++-------------
 drivers/video/omap2/dss/dss.h          |   48 ++++
 drivers/video/omap2/dss/dss_features.c |   42 ++++
 drivers/video/omap2/dss/dss_features.h |    2 +
 4 files changed, 317 insertions(+), 161 deletions(-)

Comments

Felipe Balbi Aug. 7, 2012, 8:48 a.m. UTC | #1
Hi,

On Tue, Aug 07, 2012 at 01:57:42PM +0530, Chandrabhanu Mahapatra wrote:
> diff --git a/drivers/video/omap2/dss/dss_features.c b/drivers/video/omap2/dss/dss_features.c
> index 9387097..b8d5095 100644
> --- a/drivers/video/omap2/dss/dss_features.c
> +++ b/drivers/video/omap2/dss/dss_features.c
> @@ -567,6 +567,48 @@ static const struct omap_dss_features omap4_dss_features = {
>  	.burst_size_unit = 16,
>  };
>  
> +static const struct dispc_ops omap2_dispc_ops = {
> +	.calc_scaling		=	dispc_ovl_calc_scaling_24xx,
> +	.calc_core_clk		=	calc_core_clk_24xx,
> +	.lcd_timings_ok		=	_dispc_lcd_timings_ok_24xx,
> +	.set_lcd_timings_hv	=	_dispc_mgr_set_lcd_timings_hv_24xx,
> +};
> +
> +static const struct dispc_ops omap3_2_1_dispc_ops = {
> +	.calc_scaling		=	dispc_ovl_calc_scaling_34xx,
> +	.calc_core_clk		=	calc_core_clk_34xx,
> +	.lcd_timings_ok		=	_dispc_lcd_timings_ok_24xx,
> +	.set_lcd_timings_hv	=	_dispc_mgr_set_lcd_timings_hv_24xx,
> +};
> +
> +static const struct dispc_ops omap3_3_0_dispc_ops = {
> +	.calc_scaling		=	dispc_ovl_calc_scaling_34xx,
> +	.calc_core_clk		=	calc_core_clk_34xx,
> +	.lcd_timings_ok		=	_dispc_lcd_timings_ok_44xx,
> +	.set_lcd_timings_hv	=	_dispc_mgr_set_lcd_timings_hv_44xx,
> +};
> +
> +static const struct dispc_ops omap4_dispc_ops = {
> +	.calc_scaling		=	dispc_ovl_calc_scaling_44xx,
> +	.calc_core_clk		=	calc_core_clk_44xx,
> +	.lcd_timings_ok		=	_dispc_lcd_timings_ok_44xx,
> +	.set_lcd_timings_hv	=	_dispc_mgr_set_lcd_timings_hv_44xx,
> +};
> +
> +void dispc_init_ops(const struct dispc_ops *ops)
> +{
> +	if (cpu_is_omap24xx()) {
> +		ops = &omap2_dispc_ops;
> +	} else if (cpu_is_omap34xx()) {
> +		if (omap_rev() < OMAP3430_REV_ES3_0)
> +			ops = &omap3_2_1_dispc_ops;
> +		else
> +			ops = &omap3_3_0_dispc_ops;
> +	} else {
> +		ops = &omap4_dispc_ops;
> +	}
> +}

you're not really removing. You're moving cpu_is_* somewhere else. A
better approach, IMHO, would be to use the DSS_REVISION register to
differentiate the DSS IP itself, not the OMAP.
Tomi Valkeinen Aug. 7, 2012, 9:05 a.m. UTC | #2
On Tue, 2012-08-07 at 11:48 +0300, Felipe Balbi wrote:

> you're not really removing. You're moving cpu_is_* somewhere else. A
> better approach, IMHO, would be to use the DSS_REVISION register to
> differentiate the DSS IP itself, not the OMAP.

Unfortunately we can't, DSS_REVISION contains bogus information.

I didn't look at these patches yet, but the idea has been to move the
cpu_is_* checks to only a few central places, not scattered all over.

At some point we could perhaps either move the cpu_is_ check code to
somewhere under arch/arm, or, more probably, create our own DSS version
IDs which are found out via cpu_is or soc_is or such.

 Tomi
Felipe Balbi Aug. 7, 2012, 9:14 a.m. UTC | #3
On Tue, Aug 07, 2012 at 12:05:29PM +0300, Tomi Valkeinen wrote:
> On Tue, 2012-08-07 at 11:48 +0300, Felipe Balbi wrote:
> 
> > you're not really removing. You're moving cpu_is_* somewhere else. A
> > better approach, IMHO, would be to use the DSS_REVISION register to
> > differentiate the DSS IP itself, not the OMAP.
> 
> Unfortunately we can't, DSS_REVISION contains bogus information.
> 
> I didn't look at these patches yet, but the idea has been to move the
> cpu_is_* checks to only a few central places, not scattered all over.
> 
> At some point we could perhaps either move the cpu_is_ check code to
> somewhere under arch/arm, or, more probably, create our own DSS version
> IDs which are found out via cpu_is or soc_is or such.

Or you could use the driver_data field on the platform_device_id and
of_device_id structures for that. Something like:

static const struct platform_device_id dss_id_table[] __initconst = {
{
	{ "omap2-dss", omap2_dss_features },
	{ "omap3-dss", omap3_dss_features },
	{ "omap4-dss", omap4_dss_features },
	{ "omap5-dss", omap5_dss_features },
	{} /* Terminating entry */
};

then, on your probe, you just need to copy id->driver_data to your own
structures so you can reference them later. No need for cpu_is_* or
soc_is_* or machine_is_* anywhere.

On your platform_code, wherever you create the dss device, you need to
fix up the name though. The platform_device name should match
platform_device_id name, not platform_driver name.
Tomi Valkeinen Aug. 7, 2012, 9:27 a.m. UTC | #4
On Tue, 2012-08-07 at 12:14 +0300, Felipe Balbi wrote:

> Or you could use the driver_data field on the platform_device_id and
> of_device_id structures for that. Something like:
> 
> static const struct platform_device_id dss_id_table[] __initconst = {
> {
> 	{ "omap2-dss", omap2_dss_features },
> 	{ "omap3-dss", omap3_dss_features },
> 	{ "omap4-dss", omap4_dss_features },
> 	{ "omap5-dss", omap5_dss_features },
> 	{} /* Terminating entry */
> };
> 
> then, on your probe, you just need to copy id->driver_data to your own
> structures so you can reference them later. No need for cpu_is_* or
> soc_is_* or machine_is_* anywhere.
> 
> On your platform_code, wherever you create the dss device, you need to
> fix up the name though. The platform_device name should match
> platform_device_id name, not platform_driver name.

I've thought about that, but we need versions even for different OMAP ES
versions.

So in the device tree data we couldn't have the DSS nodes in a, say,
common omap4 file. We'd need separate DT files for each OMAP ES version.

 Tomi
Felipe Balbi Aug. 7, 2012, 9:32 a.m. UTC | #5
Hi,

On Tue, Aug 07, 2012 at 12:27:52PM +0300, Tomi Valkeinen wrote:
> On Tue, 2012-08-07 at 12:14 +0300, Felipe Balbi wrote:
> 
> > Or you could use the driver_data field on the platform_device_id and
> > of_device_id structures for that. Something like:
> > 
> > static const struct platform_device_id dss_id_table[] __initconst = {
> > {
> > 	{ "omap2-dss", omap2_dss_features },
> > 	{ "omap3-dss", omap3_dss_features },
> > 	{ "omap4-dss", omap4_dss_features },
> > 	{ "omap5-dss", omap5_dss_features },
> > 	{} /* Terminating entry */
> > };
> > 
> > then, on your probe, you just need to copy id->driver_data to your own
> > structures so you can reference them later. No need for cpu_is_* or
> > soc_is_* or machine_is_* anywhere.
> > 
> > On your platform_code, wherever you create the dss device, you need to
> > fix up the name though. The platform_device name should match
> > platform_device_id name, not platform_driver name.
> 
> I've thought about that, but we need versions even for different OMAP ES
> versions.
> 
> So in the device tree data we couldn't have the DSS nodes in a, say,
> common omap4 file. We'd need separate DT files for each OMAP ES version.

you can overwrite attributes on your board dts file, right ? Meaning
that e.g. omap4.dtsi would have:

dss1: dss@xxxx {
	compatible = "ti, omap4-dss";
};


then on omap4-based-board.dts:

&dss1 {
	compatible = "ti,omap4-based-board-dss";
};

or something similar. Isn't that doable ? Benoit, would this work on
DTS ?
Tomi Valkeinen Aug. 7, 2012, 9:57 a.m. UTC | #6
On Tue, 2012-08-07 at 12:32 +0300, Felipe Balbi wrote:
> Hi,
> 
> On Tue, Aug 07, 2012 at 12:27:52PM +0300, Tomi Valkeinen wrote:
> > On Tue, 2012-08-07 at 12:14 +0300, Felipe Balbi wrote:
> > 
> > > Or you could use the driver_data field on the platform_device_id and
> > > of_device_id structures for that. Something like:
> > > 
> > > static const struct platform_device_id dss_id_table[] __initconst = {
> > > {
> > > 	{ "omap2-dss", omap2_dss_features },
> > > 	{ "omap3-dss", omap3_dss_features },
> > > 	{ "omap4-dss", omap4_dss_features },
> > > 	{ "omap5-dss", omap5_dss_features },
> > > 	{} /* Terminating entry */
> > > };
> > > 
> > > then, on your probe, you just need to copy id->driver_data to your own
> > > structures so you can reference them later. No need for cpu_is_* or
> > > soc_is_* or machine_is_* anywhere.
> > > 
> > > On your platform_code, wherever you create the dss device, you need to
> > > fix up the name though. The platform_device name should match
> > > platform_device_id name, not platform_driver name.
> > 
> > I've thought about that, but we need versions even for different OMAP ES
> > versions.
> > 
> > So in the device tree data we couldn't have the DSS nodes in a, say,
> > common omap4 file. We'd need separate DT files for each OMAP ES version.
> 
> you can overwrite attributes on your board dts file, right ? Meaning
> that e.g. omap4.dtsi would have:
> 
> dss1: dss@xxxx {
> 	compatible = "ti, omap4-dss";
> };
> 
> 
> then on omap4-based-board.dts:
> 
> &dss1 {
> 	compatible = "ti,omap4-based-board-dss";
> };
> 
> or something similar. Isn't that doable ? Benoit, would this work on
> DTS ?
> 

Yes, they can be overridden, but I think it's still quite difficult to
manage. DSS is modeled with multiple blocks, also in DT data. So you'd
need to override multiple entries.

And it'd also require a new dts file for each version of your board with
different ES version.

Also, I don't really see why this information would need to be present
in the DT data (especially as it's not simple). It's all trivially found
out in the code, by using cpu_is & soc_is.

 Tomi
Felipe Balbi Aug. 7, 2012, 10:27 a.m. UTC | #7
Hi,

On Tue, Aug 07, 2012 at 12:57:16PM +0300, Tomi Valkeinen wrote:
> On Tue, 2012-08-07 at 12:32 +0300, Felipe Balbi wrote:
> > Hi,
> > 
> > On Tue, Aug 07, 2012 at 12:27:52PM +0300, Tomi Valkeinen wrote:
> > > On Tue, 2012-08-07 at 12:14 +0300, Felipe Balbi wrote:
> > > 
> > > > Or you could use the driver_data field on the platform_device_id and
> > > > of_device_id structures for that. Something like:
> > > > 
> > > > static const struct platform_device_id dss_id_table[] __initconst = {
> > > > {
> > > > 	{ "omap2-dss", omap2_dss_features },
> > > > 	{ "omap3-dss", omap3_dss_features },
> > > > 	{ "omap4-dss", omap4_dss_features },
> > > > 	{ "omap5-dss", omap5_dss_features },
> > > > 	{} /* Terminating entry */
> > > > };
> > > > 
> > > > then, on your probe, you just need to copy id->driver_data to your own
> > > > structures so you can reference them later. No need for cpu_is_* or
> > > > soc_is_* or machine_is_* anywhere.
> > > > 
> > > > On your platform_code, wherever you create the dss device, you need to
> > > > fix up the name though. The platform_device name should match
> > > > platform_device_id name, not platform_driver name.
> > > 
> > > I've thought about that, but we need versions even for different OMAP ES
> > > versions.
> > > 
> > > So in the device tree data we couldn't have the DSS nodes in a, say,
> > > common omap4 file. We'd need separate DT files for each OMAP ES version.
> > 
> > you can overwrite attributes on your board dts file, right ? Meaning
> > that e.g. omap4.dtsi would have:
> > 
> > dss1: dss@xxxx {
> > 	compatible = "ti, omap4-dss";
> > };
> > 
> > 
> > then on omap4-based-board.dts:
> > 
> > &dss1 {
> > 	compatible = "ti,omap4-based-board-dss";
> > };
> > 
> > or something similar. Isn't that doable ? Benoit, would this work on
> > DTS ?
> > 
> 
> Yes, they can be overridden, but I think it's still quite difficult to
> manage. DSS is modeled with multiple blocks, also in DT data. So you'd
> need to override multiple entries.
> 
> And it'd also require a new dts file for each version of your board with
> different ES version.
> 
> Also, I don't really see why this information would need to be present
> in the DT data (especially as it's not simple). It's all trivially found
> out in the code, by using cpu_is & soc_is.

Fair enough. I just think we should remove all cpu_is_* and soc_is_*
from drivers.
Tomi Valkeinen Aug. 7, 2012, 10:52 a.m. UTC | #8
Hi,

On Tue, 2012-08-07 at 13:57 +0530, Chandrabhanu Mahapatra wrote:
> The cpu_is checks have been removed from DISPC providing it a much generic and
> cleaner interface. The OMAP version and revision specific functions are
> initialized by dispc_ops structure in dss features.

I think this needs some changes. I think our general approach to these
version differences should be such that the component in question (dispc
in this case) should ask from somewhere what the DSS version is, and
then using that version, handle the different versions internally.

What that means related to this patch is that we should keep all those
functions static, and initialize the dispc_ops structure inside dispc.c.

However, we don't have any such "DSS version" yet. I have previously
wanted to move the cpu_is checks to one place (dss_features), but I
think it's probably cleaner if we allow cpu_is checks in the other files
also. However, there should be only one place in the file where those
should be.

So I think we should have something like this, called from
omap_dispchw_probe():

static void dispc_init_features(void)
{
	if (cpu_is_foo())
		setup features for this omap;
	else if (...)
		...
}

This would setup the ops, or whatever is needed. This function would be
the only place in dispc.c that contains cpu_is checks.

 Tomi
Tomi Valkeinen Aug. 7, 2012, 10:57 a.m. UTC | #9
On Tue, 2012-08-07 at 13:27 +0300, Felipe Balbi wrote:
> Hi,

> > Yes, they can be overridden, but I think it's still quite difficult to
> > manage. DSS is modeled with multiple blocks, also in DT data. So you'd
> > need to override multiple entries.
> > 
> > And it'd also require a new dts file for each version of your board with
> > different ES version.
> > 
> > Also, I don't really see why this information would need to be present
> > in the DT data (especially as it's not simple). It's all trivially found
> > out in the code, by using cpu_is & soc_is.
> 
> Fair enough. I just think we should remove all cpu_is_* and soc_is_*
> from drivers.

I agree, that's part of the idea here. Instead of having cpu_is checks
all around, we'll have them in only certain centralized places. Then
it's easier to implement a way to move them totally out of the driver,
when we come up with a good idea how to accomplish that.

 Tomi
Tony Lindgren Aug. 7, 2012, 11:14 a.m. UTC | #10
* Tomi Valkeinen <tomi.valkeinen@ti.com> [120807 03:57]:
> On Tue, 2012-08-07 at 13:27 +0300, Felipe Balbi wrote:
> > Hi,
> 
> > > Yes, they can be overridden, but I think it's still quite difficult to
> > > manage. DSS is modeled with multiple blocks, also in DT data. So you'd
> > > need to override multiple entries.
> > > 
> > > And it'd also require a new dts file for each version of your board with
> > > different ES version.
> > > 
> > > Also, I don't really see why this information would need to be present
> > > in the DT data (especially as it's not simple). It's all trivially found
> > > out in the code, by using cpu_is & soc_is.
> > 
> > Fair enough. I just think we should remove all cpu_is_* and soc_is_*
> > from drivers.
> 
> I agree, that's part of the idea here. Instead of having cpu_is checks
> all around, we'll have them in only certain centralized places. Then
> it's easier to implement a way to move them totally out of the driver,
> when we come up with a good idea how to accomplish that.

Yes limiting them to driver init and setting some driver specific flags
based on that is a good start. But eventually no drivers should have
those checks.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Chandrabhanu Mahapatra Aug. 7, 2012, 12:22 p.m. UTC | #11
On Tuesday 07 August 2012 04:22 PM, Tomi Valkeinen wrote:
> Hi,
>
> On Tue, 2012-08-07 at 13:57 +0530, Chandrabhanu Mahapatra wrote:
>> The cpu_is checks have been removed from DISPC providing it a much generic and
>> cleaner interface. The OMAP version and revision specific functions are
>> initialized by dispc_ops structure in dss features.
> I think this needs some changes. I think our general approach to these
> version differences should be such that the component in question (dispc
> in this case) should ask from somewhere what the DSS version is, and
> then using that version, handle the different versions internally.
>
> What that means related to this patch is that we should keep all those
> functions static, and initialize the dispc_ops structure inside dispc.c.
>
> However, we don't have any such "DSS version" yet. I have previously
> wanted to move the cpu_is checks to one place (dss_features), but I
> think it's probably cleaner if we allow cpu_is checks in the other files
> also. However, there should be only one place in the file where those
> should be.
>
> So I think we should have something like this, called from
> omap_dispchw_probe():
>
> static void dispc_init_features(void)
> {
> 	if (cpu_is_foo())
> 		setup features for this omap;
> 	else if (...)
> 		...
> }
>
> This would setup the ops, or whatever is needed. This function would be
> the only place in dispc.c that contains cpu_is checks.
>
>  Tomi
>
Yes, this seems good to me. Its better to have all cpu_is checks of
dispc.c and dss.c in their probe functions and by the time
implementation of DSS version come to picture these can be move to dss
features or any other place appropriate. But how to handle OMAP revision
specific functions. Will Dss version handle that too, as like DSS1,
DSS1.1, DSS1.1.1, DSS2 say?
Tomi Valkeinen Aug. 7, 2012, 1 p.m. UTC | #12
On Tue, 2012-08-07 at 17:52 +0530, Chandrabhanu Mahapatra wrote:

> Yes, this seems good to me. Its better to have all cpu_is checks of
> dispc.c and dss.c in their probe functions and by the time
> implementation of DSS version come to picture these can be move to dss
> features or any other place appropriate. But how to handle OMAP revision
> specific functions. Will Dss version handle that too, as like DSS1,
> DSS1.1, DSS1.1.1, DSS2 say?

Yes, something like that.

I'm not sure how the version numbers should be created, though. If we
had only OMAP, we could use the OMAP version as the DSS major version,
and increasing minor version whenever the DSS is changed. So DSS3.2
could be the DSS used on OMAP3 SoCs, and it's the second revision,
meaning that early OMAP3 versions would have had an different DSS
version.

However, the same DSS is used for other SoCs also, so the versioning is
not that simple. I guess the other SoCs still always use a DSS that is
designed for OMAP, so the major version would be solved by that. For
example, AM3xxx DSS version would be DSS3.x. Minor version is more
tricky, because OMAP and AM3 DSS can evolve separately, which means that
a higher minor version may not contain the fix that exists in a lower
minor version.

Well, at least currently the AM3xxx's DSS is almost the same as OMAP3's,
so perhaps I'm worrying too much.

 Tomi
diff mbox

Patch

diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index 5b289c5..ad63302 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -101,6 +101,8 @@  static struct {
 	bool		ctx_valid;
 	u32		ctx[DISPC_SZ_REGS / sizeof(u32)];
 
+	const struct dispc_ops *ops;
+
 #ifdef CONFIG_OMAP2_DSS_COLLECT_IRQ_STATS
 	spinlock_t irq_stats_lock;
 	struct dispc_irq_stats irq_stats;
@@ -1939,7 +1941,18 @@  static unsigned long calc_core_clk_five_taps(enum omap_channel channel,
 	return core_clk;
 }
 
-static unsigned long calc_core_clk(enum omap_channel channel, u16 width,
+unsigned long calc_core_clk_24xx(enum omap_channel channel, u16 width,
+		u16 height, u16 out_width, u16 out_height)
+{
+	unsigned long pclk = dispc_mgr_pclk_rate(channel);
+
+	if (height > out_height && width > out_width)
+		return pclk * 4;
+	else
+		return pclk * 2;
+}
+
+unsigned long calc_core_clk_34xx(enum omap_channel channel, u16 width,
 		u16 height, u16 out_width, u16 out_height)
 {
 	unsigned int hf, vf;
@@ -1958,25 +1971,163 @@  static unsigned long calc_core_clk(enum omap_channel channel, u16 width,
 		hf = 2;
 	else
 		hf = 1;
-
 	if (height > out_height)
 		vf = 2;
 	else
 		vf = 1;
 
-	if (cpu_is_omap24xx()) {
-		if (vf > 1 && hf > 1)
-			return pclk * 4;
-		else
-			return pclk * 2;
-	} else if (cpu_is_omap34xx()) {
-		return pclk * vf * hf;
-	} else {
-		if (hf > 1)
-			return DIV_ROUND_UP(pclk, out_width) * width;
-		else
-			return pclk;
+	return pclk * vf * hf;
+}
+
+unsigned long calc_core_clk_44xx(enum omap_channel channel, u16 width,
+		u16 height, u16 out_width, u16 out_height)
+{
+	unsigned long pclk = dispc_mgr_pclk_rate(channel);
+
+	if (width > out_width)
+		return DIV_ROUND_UP(pclk, out_width) * width;
+	else
+		return pclk;
+}
+
+int dispc_ovl_calc_scaling_24xx(enum omap_channel channel,
+		const struct omap_video_timings *mgr_timings,
+		u16 width, u16 height, u16 out_width, u16 out_height,
+		enum omap_color_mode color_mode, bool *five_taps,
+		int *x_predecim, int *y_predecim, int *decim_x, int *decim_y,
+		u16 pos_x, unsigned long *core_clk)
+{
+	int error;
+	u16 in_width, in_height;
+	int min_factor = min(*decim_x, *decim_y);
+	const int maxsinglelinewidth =
+			dss_feat_get_param_max(FEAT_PARAM_LINEWIDTH);
+	*five_taps = false;
+
+	do {
+		in_height = DIV_ROUND_UP(height, *decim_y);
+		in_width = DIV_ROUND_UP(width, *decim_x);
+		*core_clk = dispc.ops->calc_core_clk(channel, in_width,
+				in_height, out_width, out_height);
+		error = (in_width > maxsinglelinewidth || !*core_clk ||
+			*core_clk > dispc_core_clk_rate());
+		if (error) {
+			if (*decim_x == *decim_y) {
+				*decim_x = min_factor;
+				++*decim_y;
+			} else {
+				swap(*decim_x, *decim_y);
+				if (*decim_x < *decim_y)
+					++*decim_x;
+			}
+		}
+	} while (*decim_x <= *x_predecim && *decim_y <= *y_predecim && error);
+
+	if (in_width > maxsinglelinewidth) {
+		DSSERR("Cannot scale max input width exceeded");
+		return -EINVAL;
+	}
+	return 0;
+}
+
+int dispc_ovl_calc_scaling_34xx(enum omap_channel channel,
+		const struct omap_video_timings *mgr_timings,
+		u16 width, u16 height, u16 out_width, u16 out_height,
+		enum omap_color_mode color_mode, bool *five_taps,
+		int *x_predecim, int *y_predecim, int *decim_x, int *decim_y,
+		u16 pos_x, unsigned long *core_clk)
+{
+	int error;
+	u16 in_width, in_height;
+	int min_factor = min(*decim_x, *decim_y);
+	const int maxsinglelinewidth =
+			dss_feat_get_param_max(FEAT_PARAM_LINEWIDTH);
+
+	do {
+		in_height = DIV_ROUND_UP(height, *decim_y);
+		in_width = DIV_ROUND_UP(width, *decim_x);
+		*core_clk = calc_core_clk_five_taps(channel, mgr_timings,
+			in_width, in_height, out_width, out_height, color_mode);
+
+		error = check_horiz_timing_omap3(channel, mgr_timings, pos_x,
+			in_width, in_height, out_width, out_height);
+
+		if (in_width > maxsinglelinewidth)
+			if (in_height > out_height &&
+						in_height < out_height * 2)
+				*five_taps = false;
+		if (!*five_taps)
+			*core_clk = dispc.ops->calc_core_clk(channel, in_width,
+					in_height, out_width, out_height);
+
+		error = (error || in_width > maxsinglelinewidth * 2 ||
+			(in_width > maxsinglelinewidth && *five_taps) ||
+			!*core_clk || *core_clk > dispc_core_clk_rate());
+		if (error) {
+			if (*decim_x == *decim_y) {
+				*decim_x = min_factor;
+				++*decim_y;
+			} else {
+				swap(*decim_x, *decim_y);
+				if (*decim_x < *decim_y)
+					++*decim_x;
+			}
+		}
+	} while (*decim_x <= *x_predecim && *decim_y <= *y_predecim && error);
+
+	if (check_horiz_timing_omap3(channel, mgr_timings, pos_x, width, height,
+		out_width, out_height)){
+			DSSERR("horizontal timing too tight\n");
+			return -EINVAL;
+	}
+
+	if (in_width > (maxsinglelinewidth * 2)) {
+		DSSERR("Cannot setup scaling");
+		DSSERR("width exceeds maximum width possible");
+		return -EINVAL;
+	}
+
+	if (in_width > maxsinglelinewidth && *five_taps) {
+		DSSERR("cannot setup scaling with five taps");
+		return -EINVAL;
 	}
+	return 0;
+}
+
+int dispc_ovl_calc_scaling_44xx(enum omap_channel channel,
+		const struct omap_video_timings *mgr_timings,
+		u16 width, u16 height, u16 out_width, u16 out_height,
+		enum omap_color_mode color_mode, bool *five_taps,
+		int *x_predecim, int *y_predecim, int *decim_x, int *decim_y,
+		u16 pos_x, unsigned long *core_clk)
+{
+	u16 in_width, in_width_max;
+	int decim_x_min = *decim_x;
+	u16 in_height = DIV_ROUND_UP(height, *decim_y);
+	const int maxsinglelinewidth =
+				dss_feat_get_param_max(FEAT_PARAM_LINEWIDTH);
+
+	in_width_max = dispc_core_clk_rate() /
+			DIV_ROUND_UP(dispc_mgr_pclk_rate(channel), out_width);
+	*decim_x = DIV_ROUND_UP(width, in_width_max);
+
+	*decim_x = *decim_x > decim_x_min ? *decim_x : decim_x_min;
+	if (*decim_x > *x_predecim)
+		return -EINVAL;
+
+	do {
+		in_width = DIV_ROUND_UP(width, *decim_x);
+	} while (*decim_x <= *x_predecim &&
+			in_width > maxsinglelinewidth && ++*decim_x);
+
+	if (in_width > maxsinglelinewidth) {
+		DSSERR("Cannot scale width exceeds max line width");
+		return -EINVAL;
+	}
+
+	*core_clk = dispc.ops->calc_core_clk(channel, in_width, in_height,
+				out_width, out_height);
+	return 0;
 }
 
 static int dispc_ovl_calc_scaling(enum omap_plane plane,
@@ -1988,12 +2139,9 @@  static int dispc_ovl_calc_scaling(enum omap_plane plane,
 {
 	struct omap_overlay *ovl = omap_dss_get_overlay(plane);
 	const int maxdownscale = dss_feat_get_param_max(FEAT_PARAM_DOWNSCALE);
-	const int maxsinglelinewidth =
-				dss_feat_get_param_max(FEAT_PARAM_LINEWIDTH);
 	const int max_decim_limit = 16;
 	unsigned long core_clk = 0;
-	int decim_x, decim_y, error, min_factor;
-	u16 in_width, in_height, in_width_max = 0;
+	int decim_x, decim_y, ret;
 
 	if (width == out_width && height == out_height)
 		return 0;
@@ -2017,118 +2165,17 @@  static int dispc_ovl_calc_scaling(enum omap_plane plane,
 	decim_x = DIV_ROUND_UP(DIV_ROUND_UP(width, out_width), maxdownscale);
 	decim_y = DIV_ROUND_UP(DIV_ROUND_UP(height, out_height), maxdownscale);
 
-	min_factor = min(decim_x, decim_y);
-
 	if (decim_x > *x_predecim || out_width > width * 8)
 		return -EINVAL;
 
 	if (decim_y > *y_predecim || out_height > height * 8)
 		return -EINVAL;
 
-	if (cpu_is_omap24xx()) {
-		*five_taps = false;
-
-		do {
-			in_height = DIV_ROUND_UP(height, decim_y);
-			in_width = DIV_ROUND_UP(width, decim_x);
-			core_clk = calc_core_clk(channel, in_width, in_height,
-					out_width, out_height);
-			error = (in_width > maxsinglelinewidth || !core_clk ||
-				core_clk > dispc_core_clk_rate());
-			if (error) {
-				if (decim_x == decim_y) {
-					decim_x = min_factor;
-					decim_y++;
-				} else {
-					swap(decim_x, decim_y);
-					if (decim_x < decim_y)
-						decim_x++;
-				}
-			}
-		} while (decim_x <= *x_predecim && decim_y <= *y_predecim &&
-				error);
-
-		if (in_width > maxsinglelinewidth) {
-			DSSERR("Cannot scale max input width exceeded");
-			return -EINVAL;
-		}
-	} else if (cpu_is_omap34xx()) {
-
-		do {
-			in_height = DIV_ROUND_UP(height, decim_y);
-			in_width = DIV_ROUND_UP(width, decim_x);
-			core_clk = calc_core_clk_five_taps(channel, mgr_timings,
-				in_width, in_height, out_width, out_height,
-				color_mode);
-
-			error = check_horiz_timing_omap3(channel, mgr_timings,
-				pos_x, in_width, in_height, out_width,
-				out_height);
-
-			if (in_width > maxsinglelinewidth)
-				if (in_height > out_height &&
-					in_height < out_height * 2)
-					*five_taps = false;
-			if (!*five_taps)
-				core_clk = calc_core_clk(channel, in_width,
-					in_height, out_width, out_height);
-			error = (error || in_width > maxsinglelinewidth * 2 ||
-				(in_width > maxsinglelinewidth && *five_taps) ||
-				!core_clk || core_clk > dispc_core_clk_rate());
-			if (error) {
-				if (decim_x == decim_y) {
-					decim_x = min_factor;
-					decim_y++;
-				} else {
-					swap(decim_x, decim_y);
-					if (decim_x < decim_y)
-						decim_x++;
-				}
-			}
-		} while (decim_x <= *x_predecim && decim_y <= *y_predecim
-			&& error);
-
-		if (check_horiz_timing_omap3(channel, mgr_timings, pos_x, width,
-			height, out_width, out_height)){
-				DSSERR("horizontal timing too tight\n");
-				return -EINVAL;
-		}
-
-		if (in_width > (maxsinglelinewidth * 2)) {
-			DSSERR("Cannot setup scaling");
-			DSSERR("width exceeds maximum width possible");
-			return -EINVAL;
-		}
-
-		if (in_width > maxsinglelinewidth && *five_taps) {
-			DSSERR("cannot setup scaling with five taps");
-			return -EINVAL;
-		}
-	} else {
-		int decim_x_min = decim_x;
-		in_height = DIV_ROUND_UP(height, decim_y);
-		in_width_max = dispc_core_clk_rate() /
-				DIV_ROUND_UP(dispc_mgr_pclk_rate(channel),
-						out_width);
-		decim_x = DIV_ROUND_UP(width, in_width_max);
-
-		decim_x = decim_x > decim_x_min ? decim_x : decim_x_min;
-		if (decim_x > *x_predecim)
-			return -EINVAL;
-
-		do {
-			in_width = DIV_ROUND_UP(width, decim_x);
-		} while (decim_x <= *x_predecim &&
-				in_width > maxsinglelinewidth && decim_x++);
-
-		if (in_width > maxsinglelinewidth) {
-			DSSERR("Cannot scale width exceeds max line width");
-			return -EINVAL;
-		}
-
-		core_clk = calc_core_clk(channel, in_width, in_height,
-				out_width, out_height);
-	}
+	ret = dispc.ops->calc_scaling(channel, mgr_timings, width, height,
+		out_width, out_height, color_mode, five_taps, x_predecim,
+		y_predecim, &decim_x, &decim_y, pos_x, &core_clk);
+	if (ret)
+		return ret;
 
 	DSSDBG("required core clk rate = %lu Hz\n", core_clk);
 	DSSDBG("current core clk rate = %lu Hz\n", dispc_core_clk_rate());
@@ -2601,27 +2648,28 @@  static bool _dispc_mgr_size_ok(u16 width, u16 height)
 		height <= dss_feat_get_param_max(FEAT_PARAM_MGR_HEIGHT);
 }
 
-static bool _dispc_lcd_timings_ok(int hsw, int hfp, int hbp,
+bool _dispc_lcd_timings_ok_24xx(int hsw, int hfp, int hbp,
 		int vsw, int vfp, int vbp)
 {
-	if (cpu_is_omap24xx() || omap_rev() < OMAP3430_REV_ES3_0) {
-		if (hsw < 1 || hsw > 64 ||
-				hfp < 1 || hfp > 256 ||
-				hbp < 1 || hbp > 256 ||
-				vsw < 1 || vsw > 64 ||
-				vfp < 0 || vfp > 255 ||
-				vbp < 0 || vbp > 255)
-			return false;
-	} else {
-		if (hsw < 1 || hsw > 256 ||
-				hfp < 1 || hfp > 4096 ||
-				hbp < 1 || hbp > 4096 ||
-				vsw < 1 || vsw > 256 ||
-				vfp < 0 || vfp > 4095 ||
-				vbp < 0 || vbp > 4095)
-			return false;
-	}
-
+	if (hsw < 1 || hsw > 64 ||
+			hfp < 1 || hfp > 256 ||
+			hbp < 1 || hbp > 256 ||
+			vsw < 1 || vsw > 64  ||
+			vfp < 0 || vfp > 255 ||
+			vbp < 0 || vbp > 255)
+		return false;
+	return true;
+}
+bool _dispc_lcd_timings_ok_44xx(int hsw, int hfp, int hbp,
+		int vsw, int vfp, int vbp)
+{
+	if (hsw < 1 || hsw > 256 ||
+			hfp < 1  || hfp > 4096 ||
+			hbp < 1  || hbp > 4096 ||
+			vsw < 1  || vsw > 256  ||
+			vfp < 0  || vfp > 4095 ||
+			vbp < 0  || vbp > 4095)
+		return false;
 	return true;
 }
 
@@ -2633,7 +2681,8 @@  bool dispc_mgr_timings_ok(enum omap_channel channel,
 	timings_ok = _dispc_mgr_size_ok(timings->x_res, timings->y_res);
 
 	if (dss_mgr_is_lcd(channel))
-		timings_ok =  timings_ok && _dispc_lcd_timings_ok(timings->hsw,
+		timings_ok =  timings_ok &&
+			dispc.ops->lcd_timings_ok(timings->hsw,
 						timings->hfp, timings->hbp,
 						timings->vsw, timings->vfp,
 						timings->vbp);
@@ -2641,6 +2690,34 @@  bool dispc_mgr_timings_ok(enum omap_channel channel,
 	return timings_ok;
 }
 
+void _dispc_mgr_set_lcd_timings_hv_24xx(enum omap_channel channel, int hsw,
+		int hfp, int hbp, int vsw, int vfp, int vbp)
+{
+	u32 timing_h, timing_v;
+
+	timing_h = FLD_VAL(hsw-1, 5, 0) | FLD_VAL(hfp-1, 15, 8) |
+			FLD_VAL(hbp-1, 27, 20);
+	timing_v = FLD_VAL(vsw-1, 5, 0) | FLD_VAL(vfp, 15, 8) |
+			FLD_VAL(vbp, 27, 20);
+
+	dispc_write_reg(DISPC_TIMING_H(channel), timing_h);
+	dispc_write_reg(DISPC_TIMING_V(channel), timing_v);
+}
+
+void _dispc_mgr_set_lcd_timings_hv_44xx(enum omap_channel channel, int hsw,
+		int hfp, int hbp, int vsw, int vfp, int vbp)
+{
+	u32 timing_h, timing_v;
+
+	timing_h = FLD_VAL(hsw-1, 7, 0) | FLD_VAL(hfp-1, 19, 8) |
+			FLD_VAL(hbp-1, 31, 20);
+	timing_v = FLD_VAL(vsw-1, 7, 0) | FLD_VAL(vfp, 19, 8) |
+			FLD_VAL(vbp, 31, 20);
+
+	dispc_write_reg(DISPC_TIMING_H(channel), timing_h);
+	dispc_write_reg(DISPC_TIMING_V(channel), timing_v);
+}
+
 static void _dispc_mgr_set_lcd_timings(enum omap_channel channel, int hsw,
 		int hfp, int hbp, int vsw, int vfp, int vbp,
 		enum omap_dss_signal_level vsync_level,
@@ -2650,25 +2727,10 @@  static void _dispc_mgr_set_lcd_timings(enum omap_channel channel, int hsw,
 		enum omap_dss_signal_edge sync_pclk_edge)
 
 {
-	u32 timing_h, timing_v, l;
+	u32 l;
 	bool onoff, rf, ipc;
 
-	if (cpu_is_omap24xx() || omap_rev() < OMAP3430_REV_ES3_0) {
-		timing_h = FLD_VAL(hsw-1, 5, 0) | FLD_VAL(hfp-1, 15, 8) |
-			FLD_VAL(hbp-1, 27, 20);
-
-		timing_v = FLD_VAL(vsw-1, 5, 0) | FLD_VAL(vfp, 15, 8) |
-			FLD_VAL(vbp, 27, 20);
-	} else {
-		timing_h = FLD_VAL(hsw-1, 7, 0) | FLD_VAL(hfp-1, 19, 8) |
-			FLD_VAL(hbp-1, 31, 20);
-
-		timing_v = FLD_VAL(vsw-1, 7, 0) | FLD_VAL(vfp, 19, 8) |
-			FLD_VAL(vbp, 31, 20);
-	}
-
-	dispc_write_reg(DISPC_TIMING_H(channel), timing_h);
-	dispc_write_reg(DISPC_TIMING_V(channel), timing_v);
+	dispc.ops->set_lcd_timings_hv(channel, hsw, hfp, hbp, vsw, vfp, vbp);
 
 	switch (data_pclk_edge) {
 	case OMAPDSS_DRIVE_SIG_RISING_EDGE:
@@ -3725,6 +3787,8 @@  static int __init omap_dispchw_probe(struct platform_device *pdev)
 
 	dispc.dss_clk = clk;
 
+	dispc_init_ops(dispc.ops);
+
 	pm_runtime_enable(&pdev->dev);
 
 	r = dispc_runtime_get();
diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
index f67afe7..d87b885 100644
--- a/drivers/video/omap2/dss/dss.h
+++ b/drivers/video/omap2/dss/dss.h
@@ -171,6 +171,21 @@  struct dss_lcd_mgr_config {
 	int lcden_sig_polarity;
 };
 
+struct dispc_ops {
+	int (*calc_scaling) (enum omap_channel channel,
+		const struct omap_video_timings *mgr_timings,
+		u16 width, u16 height, u16 out_width, u16 out_height,
+		enum omap_color_mode color_mode, bool *five_taps,
+		int *x_predecim, int *y_predecim, int *decim_x, int *decim_y,
+		u16 pos_x, unsigned long *core_clk);
+	unsigned long (*calc_core_clk) (enum omap_channel channel,
+		u16 width, u16 height, u16 out_width, u16 out_height);
+	bool (*lcd_timings_ok) (int hsw, int hfp, int hbp,
+			int vsw, int vfp, int vbp);
+	void (*set_lcd_timings_hv) (enum omap_channel channel, int hsw, int hfp,
+			int hbp, int vsw, int vfp, int vbp);
+};
+
 struct seq_file;
 struct platform_device;
 
@@ -457,6 +472,39 @@  int dispc_mgr_get_clock_div(enum omap_channel channel,
 void dispc_mgr_setup(enum omap_channel channel,
 		struct omap_overlay_manager_info *info);
 
+int dispc_ovl_calc_scaling_24xx(enum omap_channel channel,
+	const struct omap_video_timings *mgr_timings, u16 width, u16 height,
+	u16 out_width, u16 out_height, enum omap_color_mode color_mode,
+	bool *five_taps, int *x_predecim, int *y_predecim, int *decim_x,
+	int *decim_y, u16 pos_x, unsigned long *core_clk);
+int dispc_ovl_calc_scaling_34xx(enum omap_channel channel,
+	const struct omap_video_timings *mgr_timings, u16 width, u16 height,
+	u16 out_width, u16 out_height,  enum omap_color_mode color_mode,
+	bool *five_taps, int *x_predecim, int *y_predecim, int *decim_x,
+	int *decim_y, u16 pos_x, unsigned long *core_clk);
+int dispc_ovl_calc_scaling_44xx(enum omap_channel channel,
+	const struct omap_video_timings *mgr_timings, u16 width, u16 height,
+	u16 out_width, u16 out_height,  enum omap_color_mode color_mode,
+	bool *five_taps, int *x_predecim, int *y_predecim, int *decim_x,
+	int *decim_y, u16 pos_x, unsigned long *core_clk);
+
+unsigned long calc_core_clk_24xx(enum omap_channel channel, u16 width,
+		u16 height, u16 out_width, u16 out_height);
+unsigned long calc_core_clk_34xx(enum omap_channel channel, u16 width,
+		u16 height, u16 out_width, u16 out_height);
+unsigned long calc_core_clk_44xx(enum omap_channel channel, u16 width,
+		u16 height, u16 out_width, u16 out_height);
+
+bool _dispc_lcd_timings_ok_24xx(int hsw, int hfp, int hbp,
+		int vsw, int vfp, int vbp);
+bool _dispc_lcd_timings_ok_44xx(int hsw, int hfp, int hbp,
+		int vsw, int vfp, int vbp);
+
+void _dispc_mgr_set_lcd_timings_hv_24xx(enum omap_channel channel, int hsw,
+		int hfp, int hbp, int vsw, int vfp, int vbp);
+void _dispc_mgr_set_lcd_timings_hv_44xx(enum omap_channel channel, int hsw,
+		int hfp, int hbp, int vsw, int vfp, int vbp);
+
 /* VENC */
 #ifdef CONFIG_OMAP2_DSS_VENC
 int venc_init_platform_driver(void) __init;
diff --git a/drivers/video/omap2/dss/dss_features.c b/drivers/video/omap2/dss/dss_features.c
index 9387097..b8d5095 100644
--- a/drivers/video/omap2/dss/dss_features.c
+++ b/drivers/video/omap2/dss/dss_features.c
@@ -567,6 +567,48 @@  static const struct omap_dss_features omap4_dss_features = {
 	.burst_size_unit = 16,
 };
 
+static const struct dispc_ops omap2_dispc_ops = {
+	.calc_scaling		=	dispc_ovl_calc_scaling_24xx,
+	.calc_core_clk		=	calc_core_clk_24xx,
+	.lcd_timings_ok		=	_dispc_lcd_timings_ok_24xx,
+	.set_lcd_timings_hv	=	_dispc_mgr_set_lcd_timings_hv_24xx,
+};
+
+static const struct dispc_ops omap3_2_1_dispc_ops = {
+	.calc_scaling		=	dispc_ovl_calc_scaling_34xx,
+	.calc_core_clk		=	calc_core_clk_34xx,
+	.lcd_timings_ok		=	_dispc_lcd_timings_ok_24xx,
+	.set_lcd_timings_hv	=	_dispc_mgr_set_lcd_timings_hv_24xx,
+};
+
+static const struct dispc_ops omap3_3_0_dispc_ops = {
+	.calc_scaling		=	dispc_ovl_calc_scaling_34xx,
+	.calc_core_clk		=	calc_core_clk_34xx,
+	.lcd_timings_ok		=	_dispc_lcd_timings_ok_44xx,
+	.set_lcd_timings_hv	=	_dispc_mgr_set_lcd_timings_hv_44xx,
+};
+
+static const struct dispc_ops omap4_dispc_ops = {
+	.calc_scaling		=	dispc_ovl_calc_scaling_44xx,
+	.calc_core_clk		=	calc_core_clk_44xx,
+	.lcd_timings_ok		=	_dispc_lcd_timings_ok_44xx,
+	.set_lcd_timings_hv	=	_dispc_mgr_set_lcd_timings_hv_44xx,
+};
+
+void dispc_init_ops(const struct dispc_ops *ops)
+{
+	if (cpu_is_omap24xx()) {
+		ops = &omap2_dispc_ops;
+	} else if (cpu_is_omap34xx()) {
+		if (omap_rev() < OMAP3430_REV_ES3_0)
+			ops = &omap3_2_1_dispc_ops;
+		else
+			ops = &omap3_3_0_dispc_ops;
+	} else {
+		ops = &omap4_dispc_ops;
+	}
+}
+
 #if defined(CONFIG_OMAP4_DSS_HDMI)
 /* HDMI OMAP4 Functions*/
 static const struct ti_hdmi_ip_ops omap4_hdmi_functions = {
diff --git a/drivers/video/omap2/dss/dss_features.h b/drivers/video/omap2/dss/dss_features.h
index 996ffcb..d03777a 100644
--- a/drivers/video/omap2/dss/dss_features.h
+++ b/drivers/video/omap2/dss/dss_features.h
@@ -120,4 +120,6 @@  void dss_features_init(void);
 #if defined(CONFIG_OMAP4_DSS_HDMI)
 void dss_init_hdmi_ip_ops(struct hdmi_ip_data *ip_data);
 #endif
+
+void dispc_init_ops(const struct dispc_ops *ops);
 #endif